-
Notifications
You must be signed in to change notification settings - Fork 14
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 Signal from source (and enable arm64 builds) #179
Conversation
@bceverly this is a start - I'm getting an odd error about not being able to prime the |
Error currently:
|
@bceverly @kenvandine could you folks give this a try? I think I've resolved the build issues and it's getting close. |
Unfortunately I believe we're not allowed to build from source per Signal's intervention a couple of years ago. I might be misremembering but I think I recall that Canonical had to remove the Signal snap from the store while some legal work was done behind closed doors, which included committing to using the Signal client as built upstream, as-is. |
I might be misremembering tho. I found this about the problem back last year: https://forum.snapcraft.io/t/what-happened-to-signal-desktop/32119/18 |
Tagging @merlijn-sebrechts in case he can provide more memory of what the situation was and the resolution. |
Thanks for the heads up @lucyllewy - will wait for others to respond. Just noticed this build has failed - was working fine last night! I'll take a look when I get a chance later today or tomorrow. |
I have received no additional info about last year's issue, apart from "whoops, sorry for the DMCA takedown". I don't know why it initially happened, and I don't know why they ultimately accepted the Snap package. Their policy seems to be "no third party packages because users can't trust them".
I'd say to go ahead with this PR. Worst case, we just revert. Once Signal provides upstream arm64 packages, I'd like to switch back to theirs, though. |
Thanks @merlijn-sebrechts One other thing I'm not quite sure about right now, is that this builds fine for me locally, but if I use `snapcraft remote-build' to use the Launchpad builders, I get a strange error in the install process:
I'm not sure why the LP builder can't see that, but I'll do some digging and update here. |
I suspect this is an issue with the |
Yep, you're absolutely right -- there is an attempt to fix it here but no joy as yet. The offending file is obvious, but I can't seem to stop it from trying to fetch without a proxy, even if I pre-fetch the file. Will keep digging. |
Okay! I've now managed to work through the build issues on Launchpad, and have successfully launched the snap, built on Launchpad, on an amd64 machine. The process is quite messy; there are some dependencies in the @bceverly could you give this a test? /cc @kenvandine - as promised 😉 |
Many thanks for the effort on this @jnsgruk 🙏 I've briefly tested on ubuntu-asahi (arm64) and on an arm64 vm on parallels and can confirm this is working 🙏 Are there any specific features to check? |
Thank you @jnsgruk! It's working great for me on the x13s under
Mantic!!!
--
Thanks,
Bryan
…On Tue, 2023-10-31 at 10:13 -0700, Tim Holmes-Mitra wrote:
Many thanks for the effort on this @jnsgruk 🙏
I've briefly tested on ubuntu-asahi (arm64) and on an arm64 vm on
parallels and can confirm this is working 🙏 Are there any specific
features to check?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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 good to land, and I very much appreciate the inline comments which will help greatly during the ongoing maintenance. One thing we need to consider here is the ongoing need to update various parts as signal has additional releases. It doesn't look like landing this PR will break the automated update check, and if a dependent part does need to be updated it should get caught in the test-snap-can-build action. We just need to understand we're increasing the potential need for human intervention during automated maintenance.
Thanks for your effort on this and it's a very useful reference for other similar electron snaps!
Thanks! Yeh that's a good point; I've opened #181 which builds on this PR, and augments the CI to handle this automatically. |
Is the arm64 supported upstream? |
Just chatted about this with Joe and Im going to +1 this change. Merge at your discretion. |
Thanks, Alan! I've also submitted a PR to the upstream repos of those node deps that need patching - hopefully we can remove that piece of complexity from the build in the future. |
Looks good to me. I can merge it now, but know that this won't actually change anything in the store. The So the next steps are to update the actions use remote-build as explained here: snapcrafters/gimp#244 I can provide the credentials so GH actions can do this. Do you want to do that in this PR or in a next one? |
Yep - I can take that on. Let's merge this and I'll do that in another PR. |
@merlijn-sebrechts added in #181 - which will be easier to review once this merges! |
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.
LGTM!
This PR switches to enable build from source for Signal Desktop - the primary motivation is to enable builds for arm64. At present Signal does not provide an upstream deb for other architectures, so this is the only viable option I see.
There is a small difficulty which I've worked around, which is that the upstream
package.json
usesnpm-run-all
to run multiple targets as part ofyarn generate
. This was causing the scriptlets to exit early, so I've broken apart those targets into their component parts - but otherwise the process is as per their documentation.Fixes #178