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

nixos/immich: init at 1.79.1 #244803

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

nixos/immich: init at 1.79.1 #244803

wants to merge 3 commits into from

Conversation

oddlama
Copy link
Contributor

@oddlama oddlama commented Jul 22, 2023

Description of changes

This is an early draft to add a package and module for Immich, a very popular self-hosted photo and video backup solution. I would appreciate any feedback on module ergonomics. So far, all important settings are already exposed in the module and the packages should build correctly.

TODOs:

  • The nixos test would require network access and could benefit from persisting the machine learning models if that is possible (otherwise this is a multi-GB download each time the test is run)
  • Uploading images / videos could invoke external tools such as ffmpeg, exiftool, ..., and at this point I haven't tested if all required tools are propagated.
  • An update script is required to also update the sub-packages and their npmDepHashes.
  • aarch64 support should be possible and should be tested.
Things done

Early draft, not everything can be tested yet.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.11 Release Notes (or backporting 23.05 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.

@AndersonTorres
Copy link
Member

Split the commit

  • one for the package
  • one for nixos module
  • one for module tests

@oddlama
Copy link
Contributor Author

oddlama commented Jul 28, 2023

updated to 1.70.0, added automatic update script, split into separate commits

@oddlama oddlama changed the title nixos/immich: init at 1.68.1 nixos/immich: init at 1.70.0 Jul 28, 2023
@qbit
Copy link
Contributor

qbit commented Aug 7, 2023

It seems nodePackages.immich is a thing, not sure if the -cli bit should be changed to use that or not?

@oddlama
Copy link
Contributor Author

oddlama commented Aug 7, 2023

It seems nodePackages.immich is a thing, not sure if the -cli bit should be changed to use that or not?

That's the same thing that can be built from source here, just downloaded from the package registry. There currently are several cli implementations, and the one packaged here is the endorsed CLI that will replace all the others in the future.

Comment on lines +81 to +84
${optionalString serverCfg.typesense.enable ''
export TYPESENSE_API_KEY=$(cat ${serverCfg.typesense.apiKeyFile})
''}
Copy link
Contributor

Choose a reason for hiding this comment

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

immich-server looks for TYPESENSE_API_KEY even if TYPESENSE_ENABLE=false, so when cfg.server.typesense.enable = false, immich-server won't start.

@mattmelling
Copy link
Contributor

I can get as far as the admin user sign up screen, hitting sign up causes an xhr to fail with a 500

@Atemu
Copy link
Member

Atemu commented Aug 23, 2023

This needs a rebase as it does not build with the newer versions of its dependencies we have in master right now.

@oddlama
Copy link
Contributor Author

oddlama commented Aug 23, 2023

Rebased and updated to 1.74.0. Beware that it doesn't build at this moment, because some transitive python dependencies are pulling in different versions of protobuf (which to my knowledge would require updates in nixpkgs before this will work again).

@oddlama oddlama changed the title nixos/immich: init at 1.70.0 nixos/immich: init at 1.74.0 Aug 25, 2023
@oddlama oddlama changed the title nixos/immich: init at 1.74.0 nixos/immich: init at 1.75.1 Aug 26, 2023
@oddlama
Copy link
Contributor Author

oddlama commented Sep 28, 2023

@oddlama Are you still working on this? I'm interested in using thins on Nixos

Yes of course, I'll keep updating this from time to time.
Due to the protobuf 3->4 complications I wanted to let the ecosystem settle a bit before continuing. I will push an update in the next days.

I'm mostly requiring feedback on the module ergonomics at this stage. To get this out of draft stage though we'll have to wait a bit longer since there will be some significant changes to immich (possibly typesense -> pgvector) that I would like to await before pulling this into master. And since there are no comprehensive e2e tests in immich yet, I'm still not sure whether I might have missed a runtime dependency (Possibly ffmpeg and recoding stuff, hardware acceleration, ...).

@dotlambda
Copy link
Member

Due to the protobuf 3->4 complications I wanted to let the ecosystem settle a bit before continuing. I will push an update in the next days.

Isn't that solved by my comment?

@oddlama
Copy link
Contributor Author

oddlama commented Sep 28, 2023

Isn't that solved by my comment?

Possibly, I haven't tested it yet. Did you run all tests of all dependencies to see whether any of these fails due to the different protobuf version? protobuf is known to cause issues when changing major versions, so I'm not immediately sure that all dependencies support it.

@dotlambda
Copy link
Member

Did you run all tests of all dependencies to see whether any of these fails due to the different protobuf version?

No.

protobuf is known to cause issues when changing major versions, so I'm not immediately sure that all dependencies support it.

That's why the solution is going back to the older version.

@oddlama
Copy link
Contributor Author

oddlama commented Oct 1, 2023

Warning

I've tried updating this to v1.79.1, but ran into issues with the server's updated sharp dependency, which now tries to download stuff at install time which is prohibited by the sandbox. Since this is done in the dependency download step in buildNpmPackage, there is no easy workaround at this time. (Or at least none that I am aware of).

I'll have to revisit this later. If you know a workaround, please let me know.

npm ERR! code 1
npm ERR! path /build/server/node_modules/sharp
npm ERR! command failed
npm ERR! command sh -c (node install/libvips && node install/dll-copy && prebuild-install) || (node install/can-compile && node-gyp rebuild && node install/dll-copy)
npm ERR! sharp: Downloading https://github.com/lovell/sharp-libvips/releases/download/v8.14.5/libvips-8.14.5-linux-x64.tar.br
npm ERR! sharp: Please see https://sharp.pixelplumbing.com/install for required dependencies
npm ERR! sharp: Installation error: getaddrinfo EAI_AGAIN github.com

npm ERR! A complete log of this run can be found in: /build/cache/_logs/2023-10-01T00_51_29_351Z-debug-0.log
  • The protobuf issues seem to have been solved automatically, as I initially expected
  • Rebased on nixpkgs/master and updated to immich v1.79.1
  • Added packages for new required dependencies clip-server and open-clip-torch
  • All packages except the server are building, but beware that the nixos tests could not run yet.

If you want to test this PR right now, please refer to the previous version.

@oddlama oddlama changed the title nixos/immich: init at 1.75.1 nixos/immich: init at 1.79.1 Oct 1, 2023
@christoph-heiss
Copy link
Contributor

christoph-heiss commented Oct 17, 2023

Warning I've tried updating this to v1.79.1, but ran into issues with the server's updated sharp dependency, which now tries to download stuff at install time which is prohibited by the sandbox. Since this is done in the dependency download step in buildNpmPackage, there is no easy workaround at this time. (Or at least none that I am aware of).

I'll have to revisit this later. If you know a workaround, please let me know.

One thing that can be done is to pin (by overriding) the version of libvips to exactly the version it wants, something like this should work:

vips' = pkgs.vips.overrideAttrs (_: rec {
  version = "8.14.5";
  src = pkgs.fetchFromGitHub {
    owner = "libvips";
    repo = "libvips";
    rev = "v${version}";
    hash = "sha256-TzuXYKzTSptYb1fLOFzfsvsN4NWsmNfB6LUPVpaeEAg=";
    # Remove unicode file names which leads to different checksums on HFS+
    # vs. other filesystems because of unicode normalisation.
    postFetch = ''
      rm -r $out/test/test-suite/images/
    '';
  };
});

IMHO completely insane that libvips tries to download something during build time.

Edit: Can confirm that the following patch lets the current branch build again, just tried it:

diff --git a/pkgs/servers/immich/default.nix b/pkgs/servers/immich/default.nix
index 3c2b14f4bbbd..e9b3284b8ba0 100644
--- a/pkgs/servers/immich/default.nix
+++ b/pkgs/servers/immich/default.nix
@@ -230,6 +230,21 @@ let
       inherit python;
     };
   };
+
+  vips' = vips.overrideAttrs (_: rec {
+    version = "8.14.5";
+    src = pkgs.fetchFromGitHub {
+      owner = "libvips";
+      repo = "libvips";
+      rev = "v${version}";
+      hash = "sha256-fG3DTP+3pO7sbqR/H9egJHU3cLKPU4Jad6qxcQ9evNw=";
+      # Remove unicode file names which leads to different checksums on HFS+
+      # vs. other filesystems because of unicode normalisation.
+      postFetch = ''
+        rm -r $out/test/test-suite/images/
+      '';
+    };
+  });
 in
 buildNpmPackage' {
   pname = "immich";
@@ -247,7 +262,7 @@ buildNpmPackage' {
     ffmpeg
     imagemagick
     libraw
-    vips # Required for sharp
+    vips' # Required for sharp
   ];
 
   # Required because vips tries to write to the cache dir

@Scrumplex
Copy link
Member

The recent release of immich 1.88.x merged the -web component into the backend service. So this module can probably be simplified now.

See immich-app/immich#5086

@pinpox
Copy link
Member

pinpox commented Nov 29, 2023

The recent release of immich 1.88.x merged the -web component into the backend service. So this module can probably be simplified now.

@oddlama I had a look at updating your module to incorporate this, but didn't manage to make it work. It seems the immich-web systemd service can be fully removed and it's config merged with the immich-server config. If you get a version running, I can offer help with testing.

@Scrumplex
Copy link
Member

Yet another upcoming upstream change! Typesense will be dropped in favor of Postgres with the pgvecto.rs extension. As far as I can tell, the latter is not packaged yet

See https://github.com/immich-app/immich/releases/tag/v1.90.2

@Atemu
Copy link
Member

Atemu commented Dec 9, 2023

I'm slowly starting to question whether we should bother packaging at the speed at which upstream moves...

@Scrumplex
Copy link
Member

I'm slowly starting to question whether we should bother packaging at the speed at which upstream moves...

I am personally very interested in this, as I don't really like my current Docker-Compose and previously Helm deployments of Immich.

At least upstream changes mostly reduce complexity, although the Postgres extension does feel a bit more annoying to implement than having a separate daemon

@oddlama
Copy link
Contributor Author

oddlama commented Dec 9, 2023

Sorry for the slow update on this, I'm unfortunately quite busy at this time of year :/

I'm slowly starting to question whether we should bother packaging at the speed at which upstream moves...

I agree, this is becoming a concern more than I initially assumed. I've been in close contact with the immich devs for some time, so I've been aware of these big changes already, but they are still pushing out new stuff so insanely fast that it is hard to keep up.

Their architecture is just very much in motion right now and their development process (while very effective) is just docker-centric. That means there are quite some things in immich which assume exactly those conditions and that makes the module development a bit rough sometimes 😅. The current lack of E2E tests (which they are working on) means that I'll only discover these cases at runtime.

What concerns me most regarding this module is that each time they change something about their architecture (which is often) it will likely cause downstream breaking changes for users of this module.

I am personally very interested in this, as I don't really like my current Docker-Compose and previously Helm deployments of Immich.

Me too (obviously :P). But as long as native deployments are not a core goal of immich this will mean a lot of work and breakages on the NixOS side - and if I understand correctly that won't be the case for a long time, if ever.

Maybe the way to go forward here is to convert this draft to its own immich-nixos project that we can update more frequently and easily by ad-hoc packaging what is needed. That way we don't have to wait until all dependencies are available in NixOS proper. If anyone has their own thoughts on this matter, please share them :)

I'll definitely try to get all the changes in here sometime, but it'll take some work.

@christoph-heiss
Copy link
Contributor

Maybe the way to go forward here is to convert this draft to its own immich-nixos project that we can update more frequently and easily by ad-hoc packaging what is needed. That way we don't have to wait until all dependencies are available in NixOS proper. If anyone has their own thoughts on this matter, please share them :)

That definitely sounds like a idea to pursue. I'd also would very much like to deploy/use Immich and be ready to help with such an effort.

You already listed a bunch of good reason for this, and having the possibility to move fast (just like upstream) would make things a lot easier IMHO. In addition to that, it'd enable much easier collaboration on the effort to keep up with upstream and provide a working version.

If, in the future, the rapid development with breakages all the time slows down a bit, it can still be integrated in nixpkgs after all.

Just my 2ct.

@diogotcorreia
Copy link
Member

For those running Immich inside Docker but with PostgreSQL under NixOS, to update to 1.91.0 you need the pgvecto.rs PostgreSQL extension.

I've managed to (kinda) package it with the release binary (I did not try to build from source, seems to be a bit complicated because you need cargo-pgrx).
Here is the commit in my config updating to 1.91.0 if you want to copy the package: diogotcorreia/dotfiles@7676201
Suggestions are welcome!

@jvanbruegge
Copy link
Contributor

pgvecto.rs is now in nixpkgs: #281192
Would be nice to have immich as well

@bhankas
Copy link
Contributor

bhankas commented Mar 22, 2024

Since there seems to be an agreement on separate project being better suited for now, should we ask for a repo under nix-community? Ideally that would be better, but if we don't want that to be a blocker, we can always get started with any of the contributor's github and then transfer ownership. What say?

@Patryk27
Copy link
Member

should we ask for a repo under nix-community

I think having flake.nix inside the official repository would be more useful, if upstream is willing.

@bhankas
Copy link
Contributor

bhankas commented Mar 22, 2024

Since upstream devs don't use Nix for development (otherwise they'd already have a *.nix file), it would be undue burden on them to either keep the flake updated, or keep reviewing+accepting PRs for it, or provide someone full commit rights. I believe that is not a reasonable ask.

Besides, the idea behind separate repo for nixospackage+module is to be detached from both up streams (immich and nixpkgs) so we can keep working on the module itself.

Combined with f-droid allowing exact version of app to be and remain installed, both client and server can stay in sync until the package+module gets updated for upstream immich changes.

@adamcstephens
Copy link
Contributor

Since there seems to be an agreement on separate project being better suited for now, should we ask for a repo under nix-community? Ideally that would be better, but if we don't want that to be a blocker, we can always get started with any of the contributor's github and then transfer ownership. What say?

Just create a repo, start building and invite maintainers if you want. No need for it to start under nix-community.

@pinpox
Copy link
Member

pinpox commented Mar 23, 2024

Since there seems to be an agreement on separate project being better suited for now, should we ask for a repo under nix-community? Ideally that would be better, but if we don't want that to be a blocker, we can always get started with any of the contributor's github and then transfer ownership. What say?

Just create a repo, start building and invite maintainers if you want. No need for it to start under nix-community.

Feel free to ping or invite me, I'd like to test this.

@PowerUser64
Copy link
Contributor

An example of this being done in the past is when the cemu emulator was made open-source https://github.com/yboettcher/CemuFlake.

The upstream project was changing fast because of growing pains (running on linux for the first time), and there was a naming conflict with cemu-ti (formerly just called cemu) in our repo, so this flake was made in the meantime. Sounds like similar action would work well here too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet