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

build(linux): Add support for openSUSE Tumbleweed #3676

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gw3583
Copy link

@gw3583 gw3583 commented Feb 20, 2025

Description

Add support for building on openSUSE Tumbleweed

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

@gw3583
Copy link
Author

gw3583 commented Feb 20, 2025

This is a first pass. It successfully builds an RPM on an opensuse tumbleweed fresh install, and I can install it on a fresh opensuse VM and successfully stream from it. Not sure if there's anything obvious I've missed though.

@ReenigneArcher ReenigneArcher changed the title Add support for building on openSUSE Tumbleweed build(linux): Add support for openSUSE Tumbleweed Feb 20, 2025
Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

I've left some comments on the code. In addition here are a couple more.

  1. If we add a new CMAKE arg it should be added the options.cmake file
  2. We don't actually use this linux_build.sh script for rpms anymore (I've only left it for self compiling)
  3. I think we can support openSuse through our copr repo builds. If you can add whatever conditional logic you need to this spec (https://github.com/LizardByte/Sunshine/blob/master/packaging/linux/fedora/Sunshine.spec), I can enable those chroots in copr. I know this is a bit harder to test locally, so it's fine if we have to go through a few iterations of CI runs in the PR.

Comment on lines +279 to +281
elif [ "$distro" == "opensuse" ]; then
# avoid using system packages on openSUSE - boost and doxygen versions cause errors
return 1
Copy link
Member

Choose a reason for hiding this comment

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

This part of the code is just supposed to check what the installed version is.

@@ -283,6 +314,8 @@ function run_install() {
"-DSUNSHINE_ENABLE_DRM=ON"
)

cmake_args+=("-DSUNSHINE_BUILD_DISTRO='${distro}'")
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how I feel about this cmake arg yet, but it should be in the array above. It doesn't need to use the += since there is no conditional logic on whether to use it or not.

distro="opensuse"
version="tumbleweed"
package_update_command="${sudo_cmd} zypper dup"
package_install_command="${sudo_cmd} zypper in -y"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
package_install_command="${sudo_cmd} zypper in -y"
package_install_command="${sudo_cmd} zypper in -y"
cuda_version="11.8.0"
cuda_build="520.61.05"
gcc_version="11"
nvm_node=0

It appears cuda should work just fine on opensuse. https://developer.nvidia.com/cuda-11-8-0-download-archive?target_os=Linux&target_arch=x86_64&Distribution=OpenSUSE&target_version=15&target_type=runfile_local

Although, it depends on the version of gcc you can get. If gcc is too new you can set the cuda_version and cuda_build to empty like the fedora 40 one. Also please set the gcc_version to whatever version is available. I know it's not used in the script for non debian distros, but it serves as a reference for us.

openssl >= 3.0.2, \
pulseaudio-libs >= 10.0")

if (SUNSHINE_BUILD_DISTRO STREQUAL "opensuse")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (SUNSHINE_BUILD_DISTRO STREQUAL "opensuse")
if(SUNSHINE_BUILD_DISTRO STREQUAL "opensuse")

Can you also add a comment about why this behavior is needed?

@ReenigneArcher
Copy link
Member

A user just linked this on our discord, figured I'd share here just for reference: https://build.opensuse.org/package/show/games:tools/sunshine

@gw3583
Copy link
Author

gw3583 commented Feb 21, 2025

Thank you for the PR!

I've left some comments on the code. In addition here are a couple more.

1. If we add a new CMAKE arg it should be added the `options.cmake` file

2. We don't actually use this `linux_build.sh` script for rpms anymore (I've only left it for self compiling)

3. I think we can support openSuse through our copr repo builds. If you can add whatever conditional logic you need to this spec (https://github.com/LizardByte/Sunshine/blob/master/packaging/linux/fedora/Sunshine.spec), I can enable those chroots in copr. I know this is a bit harder to test locally, so it's fine if we have to go through a few iterations of CI runs in the PR.

Thanks for the feedback - will try to take a look at these in the next week or two.

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.

2 participants