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
New package: session-desktop-1.6.9 #32252
Conversation
937a08b
to
c2eb526
Compare
75eca1b
to
1d47d93
Compare
revision=1 | ||
# discontinued Electron 32-bit support: https://www.electronjs.org/blog/linux-32bit-support | ||
# aarch64 don't seems very stable for now and isn't officialy supported: https://github.com/oxen-io/session-desktop/issues/1635 | ||
archs="x86_64" |
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 think the consensus is to not restrict archs. Instead use a broken
statement (with comment). You can grep for that in the repository.
Also typo : aarch64 doesn't seem very stable for now and isn't officially supported
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 think this is fine, but is it also broken on x86_64-musl?
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.
Electron doesn't official support musl, there are patches but so it would to edit the builds scripts of Session, so it fetches the patched version of Electron, and I don't know how to do that (I could search tho)
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.
We have some Electron packages (Element, Vscode, RockerChat). You can look into those for the way to make the package use the system Electron.
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.
They use Electron 8.2.0, which is over 1y old...
( https://github.com/oxen-io/session-desktop/blob/v1.6.9/yarn.lock#L3351
https://github.com/electron/electron/releases/tag/v8.2.0 )
So using system electron isn't a option right now (We do not, and never will ship electron 8, it is long EOL and I would rather not add a package which depends on 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.
So I have spent a few hours trying to make it work with musl, but didn't get very far ^^'
I guess it'd require some patching (main problem is yea the fact it uses an old electron version. Also, just that I wasn't able to make the install script not download electron and doing AppImage which don't seems to work on musl despite heavily modifying it, but about that I may just have done something wrong tho ^^').
Besides, signal do not either support musl, or Session is a fork of it.
1d47d93
to
b183116
Compare
b183116
to
ab434a4
Compare
I had a conversation with the devs recently on Twitter. Session should build against the latest Electron do you mind giving this another go or if I were to take over? |
yes go ahead, I was getting some troubles to even just try to use system's electron ^^' |
Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it. |
General
Does it build and run successfully?