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

mautrix-signal: 0.4.3 -> unstable-2023-12-30 #277367

Merged
merged 3 commits into from
Dec 31, 2023

Conversation

niklaskorz
Copy link
Contributor

@niklaskorz niklaskorz commented Dec 28, 2023

Description of changes

mautrix-signal's latest released version v0.4.3 still uses the Python codebase
which is broken for new devices, see mautrix/signal#388.
The new Go version fixes this by using the official libsignal as a library and
can be upgraded to directly from the Python version.

Fixes #248379.

If anyone wants to try this out in combination with #277368, I also created a combined branch of these changes based on nixos-23.11: github:niklaskorz/nixpkgs/nixos-23.11-mautrix-signal.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Member

@alyaeanyx alyaeanyx left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

pkgs/by-name/li/libsignal-ffi/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/li/libsignal-ffi/package.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 28, 2023
@ofborg ofborg bot requested a review from expipiplus1 December 28, 2023 19:11
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Dec 28, 2023
@Ma27
Copy link
Member

Ma27 commented Dec 28, 2023

Nice! Since I'm currently locked out of my bridge because of the 405 error, I'll happily test this soonish! :)

Copy link
Member

@alyaeanyx alyaeanyx left a comment

Choose a reason for hiding this comment

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

LGTM

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 29, 2023
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Just two small things I noticed. Going to try it out now and hopefully I can report back soon :)

pkgs/by-name/li/libsignal-ffi/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/li/libsignal-ffi/package.nix Outdated Show resolved Hide resolved
@Ma27
Copy link
Member

Ma27 commented Dec 30, 2023

Bridge is working fine btw 👍

@niklaskorz niklaskorz changed the title mautrix-signal: 0.4.3 -> unstable-2023-12-27 mautrix-signal: 0.4.3 -> unstable-2023-12-30 Dec 30, 2023
@niklaskorz
Copy link
Contributor Author

I'm a bit unsure whether libsignal-ffi should be its own package or be part of the mautrix-signal package. Right now, this PR adds libsignal-ffi at version 0.36.1 while the latest version is 0.37.0. I tried running mautrix-signal with 0.37.0, but this leads to errors, so the version of libsignal-ffi must match unfortunately: mautrix/signal#401

@delroth delroth removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 30, 2023
@Ma27
Copy link
Member

Ma27 commented Dec 30, 2023

I'm a bit unsure whether libsignal-ffi should be its own package or be part of the mautrix-signal package

At the end of the day it's our job to provide a package set that works together, not a bunch of packages at the latest version (of course, that's not a hard rule, but something we should be keeping in mind generally).

My suggestion would be to still leave a warning in the libsignal-ffi nix expression, but as amintainer you should be pinged anyways on updates.

While you're at it, feel free to also add me to the list of maintainers for mautrix-signal (thought I already was 🤔 ) :)

@delroth delroth added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Dec 30, 2023
@niklaskorz
Copy link
Contributor Author

@Ma27 alright, done :)

@expipiplus1 any remarks from your side? do you still want to maintain the mautrix-signal package?

Copy link
Member

@Janik-Haag Janik-Haag left a comment

Choose a reason for hiding this comment

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

didin't review the package only the maintainers part. Btw please clean up your commit history a bit. the libsignal-ffi changes can probably be squashed and the mautrix-signal changes should be squashed into one commit. nixpkgs tries to have a atomic commit history.

maintainers/maintainer-list.nix Show resolved Hide resolved
@Janik-Haag Janik-Haag removed the request for review from piegamesde December 31, 2023 11:52
@delroth delroth removed the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Dec 31, 2023
@ofborg ofborg bot requested a review from Ma27 December 31, 2023 12:40
@sumnerevans sumnerevans self-requested a review December 31, 2023 15:37
@Ma27 Ma27 merged commit a7e43d8 into NixOS:master Dec 31, 2023
22 of 23 checks passed
@niklaskorz niklaskorz deleted the mautrix-signal-go branch July 10, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mautrix-signal crashes at startup
5 participants