-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
Add Mastodon package and module #78810
Conversation
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.
Beautiful!
Only thing I noticed is the package tests fail.
Failures:
1) Auth::RegistrationsController POST #create approval-based registrations with valid invite creates user
Failure/Error: expect(user.locale).to eq(accept_language)
expected: "sr-Latn"
got: "sr"
(compared using ==)
# ./spec/controllers/auth/registrations_controller_spec.rb:197:in `block (4 levels) in <top (required)>'
# ./spec/controllers/auth/registrations_controller_spec.rb:177:in `block (4 levels) in <top (required)>'
# ./spec/controllers/auth/registrations_controller_spec.rb:9:in `block (3 levels) in <top (required)>'
# ./spec/controllers/auth/registrations_controller_spec.rb:87:in `block (3 levels) in <top (required)>'
Finished in 8 minutes 47 seconds (files took 34.99 seconds to load)
2430 examples, 1 failure, 9 pending
Failed examples:
rspec ./spec/controllers/auth/registrations_controller_spec.rb:193 # Auth::RegistrationsController POST #create approval-based registrations with valid invite creates user
Randomized with seed 15353
error: command `su - tester -c 'cd mastodon; rspec --no-color'` failed (exit code 1)
Oh and Aarch64 build is failing, I suggest you just remove it from the supported architectures for now.
LoadError: /nix/store/wisk11g6scq2cbsi5ifxx10lq19j47il-mastodon-gems-3.0.1/lib/ruby/gems/2.6.0/gems/stackprof-0.2.12/lib/stackprof/stackprof.so: undefined symbol: pthread_atfork - /nix/store/wisk11g6scq2cbsi5ifxx10lq19j47il-mastodon-gems-3.0.1/lib/ruby/gems/2.6.0/gems/stackprof-0.2.12/lib/stackprof/stackprof.so
Hey. This looks good to me. Is it feasible though to add mastodons npm packages to
|
Afaik there are no official user-targeted mastodon npm packages. The webapp itself is not an npm package either, but it is exposed as a nix derivation in the nixpkgs attrset (as pkgs.mastodon.mastodon-assets). The yarn/npm dependencies used to build the client are exposed at pkgs.mastodon.mastodon-js-modules. |
@petabyteboy The idea is that we can share npm package source between different packages in nixpkgs instead of having 11.000 lines of generated nix expressions per package, which affects also evaluation time. |
Ah I see. But I doubt this would be possible since mastodon is using yarn and not npm. |
Do both not use a |
Yes and no, the mastodon repository contains a yarn.lock file with extra version information that can not be described in the original package.json format. NPM added a package-lock.json file to support the same thing, but it is incompatible with the yarn.lock format. |
So it won't capture the some indirect dependencies pinnings? Does this matter in our case though? |
I think so. Even if it is possible to package the dependencies with node2nix, the dependency versions could change every time the nix expression is generated. With such a complex application I think it is crucial to reproduce the dependencies as close to what upstream intends, and they clearly instruct users to use yarn when building the frontend. |
But on the other hand we also need to keep nixpkgs usuable. This packages adds 435.2KB of data to nixpkgs that everyone needs to download (Our node-packages.nix is 2.6M with all our packages). I get regularly complaints of people not being able to run |
Could this be leakage from your environment into the test environment? What is your |
No, this output is from the NixOS test you wrote. |
On further investigation, it appears to be a bug in the Mastodon test which happens rarely due to the use of randomization in the test. I'll report it upstream. |
After some testing I figured there should be an option to disable SMTP_LOGIN and SMTP_PASSWORD as it will block access to local smtp server without authentication.
|
b094bd6
to
3c4dad5
Compare
Does this work?
If not, please share your local smtp server configuration and steps to reproduce the problem. |
Let's continue the discussion at #83499. |
@happy-river I'm curious if you've had trouble generating user accounts? Using your module I repeatedly get "This email is invalid" messages when trying to sign up. Curious what you did? I'm using a mail server on a vps outside my lan. |
I'm using this module for a long time now, what is blocking it from getting merged ? |
Currently there's a merge conflict with all-tests.nix, likely since the line got modified in another commit. Should be easy to resolve. Besides that are there any current blockers for this PR? |
Additionally to running ./update.sh --ver v3.3.0 --patches ./resolutions.patch I did - Resolve merge conflicts in resolutions.patch - Specified buildInputs for the ruby gem openssl - Use ruby_2_7, as it is recommended in the release notes [0] - build mastodon-assets with RAILS_ENV, as it seems to be required. It would probably be nice to set it to production someday, but atm it is set to development, as production requires OTP_SECRET to be set. [0] https://github.com/tootsuite/mastodon/releases/tag/v3.3.0
I just upgraded to I hope this is ok for everyone, that I push to this PR? If anybody uses this in production, you probably need to follow the migration instructions in the release notes. |
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.
This PR and it's predecessor has been waiting for a long time and has seen a fair bit of community involvement. As far as I followed the discussion the only current issue (besides a trivial merge conflict) is the increase in closure size, due to lack of duplication in yarn.nix
.
As the metric for the problem is not the size of the Nixpkgs git repository but the size of the resulting tarball, I'd say this is a problem that could still be solved after this PR is merged.
So I'd vouch to get this merged. It is already getting used by people, it is getting maintained. Both of it would be easier if #78810 would be merged. I won't do it myself though, as I only recently aquired the commit bit and don't trust my judgement enough on this controversial PR.
I get this error when building:
Nothing I've tried is able to reproduce this other than just building the ruby-bcrypt derivation. breakpointHook works, but I can't find any more details on the issue anywhere in the filesystem. |
I had a broken |
To make this a bit easier, perhaps { mastodon, callPackage }:
mastodon.override {
version = import ./version.nix;
srcOverride = callPackage ./source.nix {};
dependenciesDir = ./.;
} |
Merge conflict :) |
As nobody else joined in my initiative to get this finally merged (either by approving or merging it), I deduce consensus is not strong enough among the commiters. Still there seems to be great interest in having this. So let's have it as an out-of-tree module? I started working on a Nix Flake mastodon-nixos, that currently somewhat mirrors the functionality of this PR. I intend to further maintain this Flake and add more functionality in the future (e.g. support for non-flake users). Goal is to move the project into the nix-community org, as I guess that'd be the right place for it. Could a member add me to the org so I can push the repo? Also I will close this PR now. It is currently based on an commit from August 2020. That means that it uses an old version of ffmpeg and imagemagick, whose should have accumulated multiple security vulnerabilities by now. No one should use this branch without a rebase in production. Feel free to reopen if anyone wants to maintain it, but I currently see not much future in it. Edit: Got it accepted into nix-community! https://github.com/nix-community/mastodon-nixos Edit: as #112898 just got merged, there is no need for an out-of-tree module anymore. I'll delete the repository. |
Oh gosh, thank you @erictapen ! A flake is a great idea |
How to fix this error?:
and
|
So as it turns out there are some people from the nix-community IRC willing to merge this. |
Motivation for this change
This PR continues the work done in #60788 by @petabyteboy, and adds these improvements:
tootctl
.pkgs.mastodon.override { ... }
work to build forks of Mastodon.package
option to the service module.import <nixpkgs>
from the source expression.Things that could still be improved:
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)