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(deps): replace libboost chrono and thread with standard chrono and thread #1364

Merged
merged 5 commits into from
Jun 26, 2023

Conversation

Era-Dorta
Copy link
Contributor

@Era-Dorta Era-Dorta commented Jun 11, 2023

Description

Sunshine crashes on Ubuntu 22.04 due a missing lib boost dependency. This PR adds the missing dependency for the deb packages.

Screenshot

Issues Fixed or Closed

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

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

@CLAassistant
Copy link

CLAassistant commented Jun 11, 2023

CLA assistant check
All committers have signed the CLA.

@ReenigneArcher
Copy link
Member

ReenigneArcher commented Jun 11, 2023

Thank you for the PR. I kind of think the solution is to switch to the standard chrono instead of boost... Unless I'm missing something.

Not to mention if we were to fix it this way, we also need to fix the rpm.

@ReenigneArcher ReenigneArcher added the question Further information is requested label Jun 11, 2023
@Era-Dorta
Copy link
Contributor Author

Thanks for lighting fast review. I didn't really look at the code much at first. I just got this error
sunshine: error while loading shared libraries: libboost_chrono.so.1.74.0: cannot open shared object file: No such file or directory
and it seemed like the obvious fix.

I now see that the code uses std:chrono and not boost:chrono. I haven't worked with C++ for a while, so I'm quite rusty at it. Does this mean that the binary somehow got built to link to libboost so the build environment for the deb packages is wrong?

@ReenigneArcher
Copy link
Member

I'm not quite sure how it linked to boost chrono as it's not even installed in the build environment.

https://github.com/LizardByte/Sunshine/blob/master/docker/ubuntu-20.04.dockerfile#L39

There might be a compile warning that doesn't fail the build and it is harder to detect.

But I think boost chrono was just used in 2 files. Probably the solution is to just switch those to standard chrono.

@Era-Dorta
Copy link
Contributor Author

Makes a lot more sense now, I've done as suggested in the last two commits.

@ReenigneArcher ReenigneArcher changed the title fix(deb): Add missing libboost-chrono to debian deps fix(deps): replace libboost-chrono with standard chrono Jun 11, 2023
@ReenigneArcher ReenigneArcher removed question Further information is requested autoupdate labels Jun 11, 2023
@Era-Dorta
Copy link
Contributor Author

Era-Dorta commented Jun 12, 2023

It looks like the boost threading doesn't like the std time. I've switched it to use std threading too. After a quick search, it seems like libboost-thread can be removed as a dependency, as this was the only place that was used. But I rather keep this PR small.

@Era-Dorta
Copy link
Contributor Author

Sorry about the crashes with the CI, I've done a manual launch on my fork and it seems to work now https://github.com/Era-Dorta/Sunshine/actions/runs/5248057310

@Era-Dorta Era-Dorta changed the title fix(deps): replace libboost-chrono with standard chrono fix(deps): replace libboost chrono and thread with standard chrono and thread Jun 12, 2023
@Era-Dorta
Copy link
Contributor Author

Is there anything else I should do to move this forwards?

@ReenigneArcher
Copy link
Member

I just need to review it, which might take some time.

There were some ci failures (cancelled) for some reason.

@Max-Might
Copy link
Contributor

@ReenigneArcher Do you think this could be released as a hotfix? Version 0.20.0 is broken for a lot of people (me including) because of this issue.

@ReenigneArcher
Copy link
Member

We don't do hot fixes and this has not been merged yet. You can manually install the missing dependency and sunshine will work fine.

@ReenigneArcher ReenigneArcher merged commit ed74492 into LizardByte:nightly Jun 26, 2023
KuleRucket pushed a commit to KuleRucket/Sunshine that referenced this pull request Jun 6, 2024
e-dong pushed a commit to e-dong/Sunshine that referenced this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants