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

Topic/upgrade boost 1.80.0 unpatched #5924

Conversation

shimpe
Copy link
Contributor

@shimpe shimpe commented Dec 1, 2022

Purpose and Motivation

fixes #5865

Types of changes

  • bugfix by upgrade to boost 1.80.0

To-do list

  • [ X] Code is tested (very quick "first" test only - I will be using this version for a while)
  • [X ] All tests are passing
  • Updated documentation
  • [ X] This PR is ready for review

@dyfer
Copy link
Member

dyfer commented Dec 1, 2022

hm, I guess that answers it... seems that MinGW still needs that patch. Thank you for checking!

@shimpe
Copy link
Contributor Author

shimpe commented Dec 1, 2022

ouch... internal compiler error in CI / Windows 64-bit MingW

D:/a/supercollider/supercollider/external_libraries/boost/boost/thread/detail/invoke.hpp:101:43: internal compiler error: in gimplify_expr, at gimplify.c:12188
return (*boost::forward(a0)).*f;

edit: oops, you saw it already

@shimpe
Copy link
Contributor Author

shimpe commented Dec 2, 2022

I've applied only the part of the patch that changes something in boost thread (which is what triggered the internal compiler error). Not sure why the MacOS shared libscsynth now failed. I don't see any clear compiler error in the logs.

2022-12-02T20:12:23.1994560Z CMake Error at /usr/local/Cellar/cmake/3.25.0/share/cmake/Modules/BundleUtilities.cmake:1128 (message):
2022-12-02T20:12:23.1996130Z error: verify_app failed
2022-12-02T20:12:23.1996920Z Call Stack (most recent call first):
2022-12-02T20:12:23.2001330Z editors/sc-ide/cmake_install.cmake:98 (verify_app)
2022-12-02T20:12:23.2002670Z editors/cmake_install.cmake:44 (include)
2022-12-02T20:12:23.2002950Z cmake_install.cmake:63 (include)
2022-12-02T20:12:23.2003100Z
2022-12-02T20:12:23.2003100Z
2022-12-02T20:12:23.2003110Z
2022-12-02T20:12:23.2572910Z ** BUILD FAILED **
2022-12-02T20:12:23.2573500Z
2022-12-02T20:12:23.2573680Z
2022-12-02T20:12:23.2573990Z The following build commands failed:
2022-12-02T20:12:23.2575490Z PhaseScriptExecution CMake\ PostBuild\ Rules /Users/runner/work/supercollider/supercollider/build/build/SuperCollider.build/Release/install.build/Script-93426DA6A4AC2C2AC9CE883C.sh (in target 'install' from project 'SuperCollider')
2022-12-02T20:12:23.2576340Z (1 failure)
2022-12-02T20:12:23.2892320Z ##[error]Process completed with exit code 65

@dyfer
Copy link
Member

dyfer commented Dec 3, 2022

Not sure why the MacOS shared libscsynth now failed. I don't see any clear compiler error in the logs.

Yeah, it's weird, it might be a random error. I'm re-running the CI to check.

@dyfer dyfer added the comp: 3rd party 3rd party libraries/dependencies label Dec 4, 2022
@dyfer
Copy link
Member

dyfer commented Dec 6, 2022

Note to self: my main concern was whether this doesn't impede compatibility with older platforms. I tested the macOS legacy build from this PR on macOS 10.10 and it still runs (server boots, File operations appear to work), i.e. it looks good in that regard (especially that the CI with older compilers still work).

I'll let others chime in before merging this, but I think this should be good to go. Also, I think we should use this "minimally patched" version as opposed to #5920.

Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

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

Upon a closer look I have a couple of requests here.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@dyfer
Copy link
Member

dyfer commented Dec 18, 2022

I think this is technically good to go, but I would like to wait before merging this to see how we deal with FluCoMa and possibly extending the plugin interface. I'd prefer to merge this once we at least have a plan to upgrade the plugin interface to accommodate FluCoMa needs (see flucoma/flucoma-sc#149)

In any case, I think this should be (eventually) merged into develop to be included in the 3.14 release, not earlier.

@dyfer dyfer mentioned this pull request Dec 18, 2022
1 task
@shimpe
Copy link
Contributor Author

shimpe commented Dec 18, 2022

I'm ok with waiting since I can anyway use my own build until it gets merged.

@dvzrv
Copy link
Member

dvzrv commented Jan 11, 2023

Since we are building against a 1.80.0 boost (system) version on Arch Linux already, I guess flucoma-sc is broken there already. 🤔

@dyfer
Copy link
Member

dyfer commented Jan 11, 2023

If Flucoma is built against the same boost version, then all is fine...
However, I hope that we'll fix the need to link to boost by plugins by extending plugin API #5939

@dvzrv
Copy link
Member

dvzrv commented Jan 11, 2023

If Flucoma is built against the same boost version, then all is fine... However, I hope that we'll fix the need to link to boost by plugins by extending plugin API #5939

Ah, alright. I'm not yet providing it in the repos. Maybe some day though!

@shimpe
Copy link
Contributor Author

shimpe commented Feb 13, 2024

@dyfer Since several people have hit the same filesystem bug, and since no one seems to be working on the plugin API (correct me if I'm wrong), does it still make sense to keep waiting?

@Spacechild1
Copy link
Contributor

does it still make sense to keep waiting?

IMO there's no reason to wait. Frankly speaking, if plugin authors are depending on private classes/functions, it's really their problem. The very point of private APIs is that we can change them whenever we want.

(To be clear, I'm all for extending the public plugin API, because I'm suffering from its limitations myself, but that's a completely different issue.)

dyfer
dyfer previously approved these changes Jun 20, 2024
Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

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

Yes, I think we should proceed, enough time has passed. Maybe once the plugins break, it'll encourage work on the plugin api...

@dyfer dyfer self-requested a review June 20, 2024 07:13
@dyfer dyfer dismissed their stale review June 20, 2024 07:14

Maybe we should go with the 1.83 PR?

@capital-G
Copy link
Contributor

Is it ok if we close this in favor of #6334 ?

@shimpe
Copy link
Contributor Author

shimpe commented Aug 30, 2024

Sure go ahead, as long it doesn't take another year. My only concern is getting the bug finally fixed :D

@shimpe shimpe closed this Aug 30, 2024
@dyfer dyfer mentioned this pull request Sep 14, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: 3rd party 3rd party libraries/dependencies
Projects
Development

Successfully merging this pull request may close these issues.

ERROR: Primitive '_FileCopy' failed. when starting scide - boost needs upgrade to higher version to fix
5 participants