sharkey wip (dont merge yet) #296697

CutestNekoAqua · 2024-03-17T16:38:36Z

Description of changes

WIP PR to show the progress of Sharkey > Nixpkgs
continuation of #259829

Things done

Add a 👍 reaction to pull requests you find important.

CutestNekoAqua · 2024-03-17T18:24:20Z

noo ofborg 💀

I forgot to swap out the maintainer info there for a second before pushing 😭

CutestNekoAqua · 2024-03-17T18:24:51Z

Info for people looking at this:
sharkey does not build on machines with 4GB or RAM or less. :/

pkgs/by-name/sh/sharkey/package.nix

CutestNekoAqua · 2024-03-17T19:06:45Z

@Weathercold can you test this for me and give me your thoughts on it? This way we can get it merged quicker*

*limiting factor will still be getting someone with commit rights to merge in the end ;P

Edit: pinging you here because your question on the old PR makes me assume you are interested in using it once its merged.

pluiedev · 2024-03-17T19:23:41Z

noo ofborg 💀

I forgot to swap out the maintainer info there for a second before pushing 😭

I was about to ask why was I pinged for this 😹 I guess this is tangentially related though since the pnpm deps code is from Vesktop

CutestNekoAqua · 2024-03-17T19:57:43Z

noo ofborg 💀
I forgot to swap out the maintainer info there for a second before pushing 😭

I was about to ask why was I pinged for this 😹 I guess this is tangentially related though since the pnpm deps code is from Vesktop

yes. this is a unholy fusion of vesktop and #161855

maybe I will do that for misskey too later down the line.. Vesktop is the first pnpm nix impl that works here tho 😸

pluiedev · 2024-03-17T20:06:20Z

Have you seen #290715? That is essentially Vesktop's pnpm deps logic packaged into a fetcher 😺

pluiedev · 2024-03-17T20:08:11Z

Also, please mark the PR as a draft if you don't want it merged or thoroughly reviewed by other people with write perms :) Change the title too to the standard format: 'sharkey: init at x.y.z'

CutestNekoAqua · 2024-03-17T20:13:02Z

Have you seen #290715? That is essentially Vesktop's pnpm deps logic packaged into a fetcher 😺

yes, but I would not like to wait for it :d

CutestNekoAqua · 2024-03-17T20:14:24Z

Also, please mark the PR as a draft if you don't want it merged or thoroughly reviewed by other people with write perms :) Change the title too to the standard format: 'sharkey: init at x.y.z'

I want it thoroughly reviewed tho! Its not marked as a draft because it should be tested already! I just dont want it to be merged because my commits dont follow the contribution guidelines right now and I will forcepush to make the changes into commits how they should be once we tested everything <3

CutestNekoAqua · 2024-03-17T20:19:42Z

I am fairly certain, that this code already 100% works and would be ready to merge, but because we did not setup a server with it yet to test if everything including AP federation works flawlessly, its not ready to merge in my eyes.

pluiedev

It's a bunch of nitpicking but honestly it looks pretty good — now we just have to figure out what needs to be done next

pluiedev · 2024-03-17T22:25:46Z

nixos/modules/services/networking/sharkey.md

+{ config, ... }:
+
+{
+  services.misskey = {


Still says misskey here

pluiedev · 2024-03-17T22:26:34Z

nixos/modules/services/networking/sharkey.md

+    ensureDatabases = [ "sharkey" ];
+    ensureUsers = [
+      {
+        name = "sharkey;


Missing closing quotation mark

- name = "sharkey; + name = "sharkey”;

pluiedev · 2024-03-17T22:27:38Z

pkgs/by-name/sh/sharkey/package.nix

+    owner = "TransFem-org";
+    repo = "Sharkey";
+    domain = "activitypub.software";
+    rev = "${finalAttrs.version}";


Nit: unnecessary quotes & interpolation

nixos/modules/services/networking/sharkey.md

vgskye · 2024-03-18T02:13:54Z

pkgs/by-name/sh/sharkey/package.nix

+      postPatch = ''
+        mv package.json package.json.orig
+        jq --raw-output ". * $pnpmPatch" package.json.orig > package.json
+      '';


ime running pnpm with --force(as in #290715) seems to be more reliable than this package.json patching in some cases involving cross-platform builds? not sure on the conditions of such a thing happening, so take with a pinch of salt

--force is better, yes. See pnpm/pnpm#7695

But I am still not very confident on this fixed-output derivation as it does not provide stable hashes across different versions of pnpm

hmm, how can we execute scripts without making it unreproducible?

sodiboo · 2024-05-26T21:03:04Z

Hey, is this still being worked on? I'm interested in hosting an instance of Sharkey on NixOS, and this seems like a good start probably. If this is abandoned, i might try to continue these efforts in a new PR.

CutestNekoAqua · 2024-05-27T20:00:50Z

Hey, is this still being worked on? I'm interested in hosting an instance of Sharkey on NixOS, and this seems like a good start probably. If this is abandoned, i might try to continue these efforts in a new PR.

its not abandoned. other things in my calendar where just taking up time more. Feel free to request changes / add yourself as second maintainer later.

sodiboo · 2024-05-28T11:35:12Z

I based some work in my own config off of this PR. I've got it running now. The modules are in the sharkey/ subdirectory of my config (permalink) with my own configuration at ../sharkey.mod.nix relative to the link. it's running live at gaysex.cloud.

CutestNekoAqua · 2024-05-28T14:39:06Z

awesome! i will look update this PR in a day or two

sodiboo · 2024-05-28T15:53:05Z

pkgs/by-name/sh/sharkey/package.nix

+
+    (
+      cd node_modules/.pnpm/node_modules/v-code-diff
+      node scripts/postinstall.js


This file is named postinstall.cjs, and it needs to have the correct extension to run.

It seems clearer to just run their postinstall script, which does actually just run this file.

Suggested change

node scripts/postinstall.js

pnpm run postinstall

sodiboo · 2024-05-28T15:55:56Z

pkgs/by-name/sh/sharkey/package.nix

+  preBuild = ''
+    export HOME=$(mktemp -d)
+    export STORE_PATH=$(mktemp -d)
+    export NODE_OPTIONS="--max_old_space_size=4096"


Is this necessary? The frontend fails to build with a very unintuitive error, which on some sources say that this flag will fix because it's out of memory. Manually running cd packages/frontend; pnpm build (so that it actually shows its output rather than being mangled by the TTY-centric "overview" interface) shows that it's actually v-code-diff failing to build, as far as i can tell. Running that install script correctly makes this flag unnecessary for me.

sodiboo · 2024-05-28T15:59:24Z

pkgs/by-name/sh/sharkey/package.nix

+        jemalloc
+        ffmpeg-headless
+        stdenv.cc.cc.lib
+      ]);


We also need to add a binary path for nodePackages.pnpm and bash, because they are used in the npm scripts of Sharkey (e.g. pnpm migrateandstart will use shell scripting && to run pnpm migrate && pnpm start; in particular, pnpm isn't qualified here so it needs to be in PATH, even if we qualify it fully for direct invocations)

Suggested change

]);

]);

binPath = lib.makeBinPath [

bash

nodePackages.pnpm

];

sodiboo · 2024-05-28T16:00:35Z

pkgs/by-name/sh/sharkey/package.nix

+
+      makeWrapper ${nodePackages.pnpm}/bin/pnpm $out/bin/sharkey \
+        --run $out/data
+        --prefix LD_LIBRARY_PATH : ${libPath} \


See above comment at the start of the install phase.

Suggested change

--prefix LD_LIBRARY_PATH : ${libPath} \

--prefix LD_LIBRARY_PATH : ${libPath} \

--prefix PATH : ${binPath} \

sodiboo · 2024-05-28T16:02:45Z

pkgs/by-name/sh/sharkey/package.nix

+      mkdir -p $out/data/packages/client
+      ln -s /var/lib/sharkey $out/data/files
+      ln -s /run/sharkey $out/data/.config
+      cp -r locales node_modules built fluent-emojis tossface-emojis sharkey-assets packages package.json pnpm-workspace.yaml $out/data


Some of these files are nonexistent. Why are we selecting them in such a fragile way?

Suggested change

cp -r locales node_modules built fluent-emojis tossface-emojis sharkey-assets packages package.json pnpm-workspace.yaml $out/data

cp -r * $out/data

sodiboo · 2024-05-28T16:03:39Z

pkgs/by-name/sh/sharkey/package.nix

+    ''
+      runHook preInstall
+
+      mkdir -p $out/data


Why data? That feels very generic and undescriptive. Why not, e.g., Sharkey (matching the actual name of the corresponding directory in a typical install)

sodiboo · 2024-05-28T16:04:23Z

pkgs/by-name/sh/sharkey/package.nix

+      # https://gist.github.com/MikaelFangel/2c36f7fd07ca50fac5a3255fa1992d1a
+
+      makeWrapper ${nodePackages.pnpm}/bin/pnpm $out/bin/sharkey \
+        --run $out/data


Needs a backslash.

Suggested change

--run $out/data

--run $out/data \

sodiboo · 2024-05-28T16:10:03Z

pkgs/by-name/sh/sharkey/package.nix

+      makeWrapper ${nodePackages.pnpm}/bin/pnpm $out/bin/sharkey \
+        --run $out/data
+        --prefix LD_LIBRARY_PATH : ${libPath} \
+        --prefix NODE_ENV : production


This is also missing a backslash. Also, why --prefix? This isn't a list.

Suggested change

--prefix NODE_ENV : production

--set-default NODE_ENV production \

sodiboo · 2024-05-28T16:16:49Z

pkgs/by-name/sh/sharkey/package.nix

+        --run $out/data
+        --prefix LD_LIBRARY_PATH : ${libPath} \
+        --prefix NODE_ENV : production
+        --add-flags migrateandstart


Should this really be always passed? Wouldn't it make more sense to not be passing it and let the consumer choose which script to invoke? e.g. a consumer might want to run just sharkey migrate when doing internal maintenance, or just sharkey start to start without a db migration. there also exists sharkey init which is mostly useless because sharkey migrate will initialize an empty DB, and there's also sharkey revert which undoes a migration, and that one is not covered by migrateandstart.

migrateandstart is a good choice for the "default command", though, so we should absolutely be using that for the systemd service.

sodiboo · 2024-05-28T16:19:20Z

pkgs/by-name/sh/sharkey/package.nix

+  '';
+
+  postBuild = ''
+    pnpm build --reporter=ndjson


This --reporter=ndjson doesn't seem to do much, because pnpm build here is just a script that immediately invokes pnpm -r build and --reported=ndjson does not get passed to it. Also, why is this postBuild? That's clearly the main build step.

sodiboo · 2024-05-28T16:21:54Z

nixos/modules/services/networking/sharkey.nix

+        ${pkgs.envsubst}/bin/envsubst -i "${configFile}" > /run/sharkey/default.yml
+        cd ${pkgs.sharkey}/data
+      '';
+      serviceConfig = {


Is this running as root?

Suggested change

serviceConfig = {

serviceConfig = {

User = "sharkey";

sodiboo · 2024-05-28T16:23:21Z

nixos/modules/services/networking/sharkey.nix

+        SystemCallFilter = "@system-service";
+        UMask = "0077";
+      };
+      environment.NODE_ENV = "production";


Why are we setting NODE_ENV here and in the wrapper?

sodiboo · 2024-05-28T16:31:06Z

pkgs/by-name/sh/sharkey/package.nix

+    (
+      cd node_modules/.pnpm/node_modules/v-code-diff
+      node scripts/postinstall.js
+    )


re2 and sharp also need install scripts to run. They use node-gyp, so we need to set the node path or else it'll try to fetch headers from nodejs.org.

Without them, the build succeeds but we get runtime errors as the binaries can't be loaded (so, the package won't actually work).

Suggested change

)

)

export npm_config_nodedir=${nodePackages.nodejs}

(

cd node_modules/.pnpm/node_modules/re2

pnpm run rebuild

)

(

cd node_modules/.pnpm/node_modules/sharp

pnpm run install

)

sharkey wip (dont merge yet)

8b40267

CutestNekoAqua mentioned this pull request Mar 17, 2024

sharkey: init at 2023.9.1.beta5 #259829

Closed

12 tasks

ofborg bot added the 8.has: package (new) label Mar 17, 2024

ofborg bot requested review from Scrumplex, getchoo, pluiedev and vgskye March 17, 2024 18:01

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Mar 17, 2024

make editorconfig happy

226f622

CutestNekoAqua force-pushed the sharkey branch from d8eaed0 to 226f622 Compare March 17, 2024 18:23

Scrumplex reviewed Mar 17, 2024

View reviewed changes

pkgs/by-name/sh/sharkey/package.nix Outdated Show resolved Hide resolved

push changes

e97d0a6

premature module testing?

8623552

github-actions bot added 6.topic: nixos 8.has: module (update) labels Mar 17, 2024

ofborg bot added the 11.by: package-maintainer label Mar 17, 2024

pluiedev suggested changes Mar 17, 2024

View reviewed changes

fix build

19755ea

CutestNekoAqua added 2 commits March 17, 2024 23:48

fix build

5359d24

manually execute postinstall script for vue thingy

b6b1454

ofborg bot removed the 10.rebuild-linux: 1 label Mar 18, 2024

vgskye reviewed Mar 18, 2024

View reviewed changes

nixos/modules/services/networking/sharkey.md Show resolved Hide resolved

vgskye reviewed Mar 18, 2024

View reviewed changes

sodiboo reviewed May 28, 2024

View reviewed changes

h7x4 added 8.has: documentation 8.has: module (new) labels Jun 1, 2024

SuperSandro2000 marked this pull request as draft June 6, 2024 11:46

feathecutie mentioned this pull request Jun 12, 2024

misskey: init at 2024.5.0 & nixos/misskey: init module #319348

Merged

20 tasks

	--prefix LD_LIBRARY_PATH : ${libPath} \
	--prefix LD_LIBRARY_PATH : ${libPath} \
	--prefix PATH : ${binPath} \

	cp -r locales node_modules built fluent-emojis tossface-emojis sharkey-assets packages package.json pnpm-workspace.yaml $out/data
	cp -r * $out/data

	--prefix NODE_ENV : production
	--set-default NODE_ENV production \

-    )
+    )
+    export npm_config_nodedir=${nodePackages.nodejs}
+    (
+      cd node_modules/.pnpm/node_modules/re2
+      pnpm run rebuild
+    )
+    (
+      cd node_modules/.pnpm/node_modules/sharp
+      pnpm run install
+    )

sharkey wip (dont merge yet) #296697

Are you sure you want to change the base?

sharkey wip (dont merge yet) #296697

Conversation

CutestNekoAqua commented Mar 17, 2024 • edited Loading

Description of changes

Things done

CutestNekoAqua commented Mar 17, 2024

CutestNekoAqua commented Mar 17, 2024

CutestNekoAqua commented Mar 17, 2024 • edited Loading

pluiedev commented Mar 17, 2024

CutestNekoAqua commented Mar 17, 2024

pluiedev commented Mar 17, 2024 • edited Loading

pluiedev commented Mar 17, 2024 • edited Loading

CutestNekoAqua commented Mar 17, 2024

CutestNekoAqua commented Mar 17, 2024

CutestNekoAqua commented Mar 17, 2024

pluiedev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sodiboo commented May 26, 2024

CutestNekoAqua commented May 27, 2024

sodiboo commented May 28, 2024 • edited Loading

CutestNekoAqua commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sodiboo May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CutestNekoAqua commented Mar 17, 2024 •

edited

Loading

CutestNekoAqua commented Mar 17, 2024 •

edited

Loading

pluiedev commented Mar 17, 2024 •

edited

Loading

pluiedev commented Mar 17, 2024 •

edited

Loading

sodiboo commented May 28, 2024 •

edited

Loading

sodiboo May 28, 2024 •

edited

Loading