-
Notifications
You must be signed in to change notification settings - Fork 6
Enable dullahan Linux builds #4
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
Conversation
…e a prebuild CEF from spotify. This will obviously miss the proprietary codecs, but it's still better than no CEF at all
Cleanup, install will duplicate files into bin/release and lib/release. Delete the shared library files in bin/release to avoid duplicate files in final archive
callumlinden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this for Linux builds only vs all builds?
callumlinden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch!
| -DCMAKE_CXX_FLAGS:STRING="$opts" \ | ||
| $(cmake_cxx_standard $LL_BUILD_RELEASE) | ||
| cmake -S . -B stage/build -DCMAKE_BUILD_TYPE=Release -G Ninja -DCMAKE_INSTALL_PREFIX=stage \ | ||
| -DUSE_SPOTIFY_CEF=TRUE -DSPOTIFY_CEF_URL=https://cef-builds.spotifycdn.com/cef_binary_118.4.1%2Bg3dd6078%2Bchromium-118.0.5993.54_linux64_beta_minimal.tar.bz2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to choose v118? That's 5+ months old now and latest version (v123) builds for me locally on Windows and macOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. For parity with Windows/OSX. Per autobuild
https://github.com/secondlife/cef/releases/download/v118.0.5993.54b/cef_bin-118.4.1_g3dd6078_chromium-118.0.5993.54-windows64-9522e46.tar.zst
Your local environment is not authoritative for me, this repository is :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny. The v123 build would be authoritative now if we were able to build the macOS in GHA.
Trivial for you to update though, as and when we are able to build it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@callumlinden or I can update it now if you want (and can give me the CEF version to target). Saves us the work later to update.
An considering the first Linux release will be beta anyway, I personally see no issue to have them with a newer CEF version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cef-builds.spotifycdn.com/cef_binary_123.0.7%2Bg6a21509%2Bchromium-123.0.6312.46_linux64_beta.tar.bz2 is the Linux build of the version I built locally. There were no interfaces changes between it and 118.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cef-builds.spotifycdn.com/cef_binary_123.0.7%2Bg6a21509%2Bchromium-123.0.6312.46_linux64_beta.tar.bz2 is the Linux build of the version I built locally. There were no interfaces changes between it and 118.
That has not been my experience - there have been changes, you cannot just simply drop a CEF v123 build into the viewer thats using v118 and then its happy days.
See this: secondlife/viewer#1099 (comment)
This in particular:
need to change how the CEF cache is managed (newer CEF versions refuse to share the same cache between several instances: this also affects cookies
I personally tested CEF v123 on firestorm a few weeks back and confirm there are issues like are described in the above link - The initial viewer splash page (login web page) loaded ok, but once in-world, trying to start an additional instance (ie: init the internal web browser failed) - (there was error being thrown in the debug console when trying to open the internal browser (I dont remember exactly what it said, it was some error relating to using a different method or something)) - either way, I suspect that viewer code base changes might be needed to use v123.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That has not been my experience - there have been changes, you cannot just simply drop a CEF v123 build into the viewer thats using v118 and then its happy days.
Interesting. Since we don't have a GHA macOS build yet, I have only given the Viewer integration a quick smoke test and it seemed to be okay. When you say "drop a CEF v123 build into the viewer", I assume you mean build the Viewer against it and not just copy over the plethora of runtime files into the right place?
need to change how the CEF cache is managed (newer CEF versions refuse to share the same cache between several instances: this also affects cookies
To the best of my knowledge, this has always been true and the reason images caching and cookie storage is to unreliable. I have some ideas about how to fix it - or at least, work around it - but I haven't yet found the time.
I think if we were to use CEF in a "one window, multiple tabs" model, these issues might go away - vs the "multiple windows" paradigm in place now.
Plenty of things to try though.
I'll make a Viewer build next week with v123 and see what I can discover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Since we don't have a GHA macOS build yet, I have only given the Viewer integration a quick smoke test and it seemed to be okay. When you say "drop a CEF v123 build into the viewer", I assume you mean build the Viewer against it and not just copy over the plethora of runtime files into the right place?
Correct, I built CEF v123 from scratch using our (FS) 3p-cef (and waited 7 years for the thing to build, lol), then rolled a new dullahan to consume it - all that worked fine - then I rolled a new build of FS which consumed the updated dullahan with CEF v123 in it - the viewer also compiled and linked fine - and I was presented with the main login webpage when the viewer started - I was smiling until I actually logged in and then tried to fire up the internal web browser - and thats when the wheels fell off the wagon.
I had not been able to test against the official LL linux viewer for obvious reasons. If you could let me know if you find anything resembling what I described when you get around to looking at it :)
Thanks!
And a mega thank you to everyone involved so far in helping to try and resurrect the LL Linux viewer (both to those filing the PR's and those at LL who are being willing to accept them and merge them in) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it now - very strange and the first time I've encountered this.
I've done a little digging and CEFInitialize() is failing in Dullahan::initCEF(). I think it's related to using the same cache/settings for each instance - looks like you're now supposed to implement CefBrowserProcessHandler::OnAlreadyRunningAppRelaunch() for this case.
I'll continue to poke tomorrow but I do wonder if now is the time to once and for all address the fact that we're not supposed to use the came cache/cookie store for multiple instances of CEF - certainly seems like it's related. No idea what that fix might be though.
|
@callumlinden it looks like this repository isn't set up to build/test PRs. Fixed in #5. Could we get that merged, update this branch to test builds? |
@bennettgoble done |
|
@Nicky-D thanks for the changes, they are released in https://github.com/secondlife/dullahan/releases/tag/v1.14.0-r2 |
Rght now there is no LL-CEF build ( see Issue 3 in CEF ) to consume.
To get around this, the build with be a bit sneaky and just use a binary from Spotify (the official CEF nightly builds)
Why no custom built CEF first? I believe this will be a futile exercise trying to attempt this using the free github runners:
As a consequence there will be no proprietary codecs; But at least there will be a Linux dullahan, which beats having no browser at all.
@callumlinden or @bennettgoble can one of you review this, merge if golden and please create a release?