nextcloud31Packages.apps.memories: package by hand to remove pre-compiled binaries; nextcloud31Packages.apps.recognize: more cleanup

[SuperSandro2000]

Also contains another cleanup for recognize from which I copied most of the code.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

---

Add a 👍 reaction to pull requests you find important.

[beardhatcode]

I tried this in a vm but I get:

❗ exiftool failed test: binary path is empty (run occ maintenance:repair or use system perl).

❗ ffmpeg preview binary not found. Thumbnail generation may not work for videos.

Memories - Administration settings - Nextcloud.pdf

But this does look cool 🙂

[Ma27]

Hmm, does this app have enough CLI tooling around, so we could write a VM test for this?
The patching in here and recognize is a little brittle, unfortunately.

---

Anyways, "requesting changes" until the issue @beardhatcode reported is sorted out.

[beardhatcode]

@SuperSandro2000 , I made a patch that makes a patch that puts makes it use the right paths: beardhatcode@3d0d447

In essence, I manually patch lib/Settings/SystemConfig.php with patch, to get the paths from the nix store.

https://github.com/beardhatcode/nixpkgs/blob/3d0d4473ae83b0bbc68df5b1813849ebc4b34b83/pkgs/servers/nextcloud/packages/apps/memories.nix#L68-L93

With that it works for me I can now watch my videos transcoded to 480p (anything more is too slow because I do not have a GPU).

The patch can likely go into patches but I did no t have time to fix that now

[SuperSandro2000]

I tried this in a vm but I get:

❗ exiftool failed test: binary path is empty (run occ maintenance:repair or use system perl).

We don't need this as exiftool from us always works.

❗ ffmpeg preview binary not found. Thumbnail generation may not work for videos.

That should be handles by ln -s ${lib.getExe ffmpeg} bin-ext/ffmpeg

Hmm, does this app have enough CLI tooling around, so we could write a VM test for this?

You need sizeable amount of pictures (200 IIRC) before it really kicks in. So with just a handful it just sits there and does not much.

And not really

 ➜ nextcloud-occ | grep memories
 memories
  memories:index                         Index the metadata in files
  memories:migrate-google-takeout        Migrate JSON metadata from Google Takeout
  memories:places-setup                  Setup reverse geocoding

[SuperSandro2000]

I didn't really want to make a module for this, so I mostly relied on the upstream fallbacks and patched them to work. I've been using this though https://github.com/NuschtOS/nixos-modules/blob/main/modules/nextcloud.nix#L106-L111

[SuperSandro2000]

@SuperSandro2000 , I made a patch that makes a patch that puts makes it use the right paths: beardhatcode@3d0d447

I don't really want to do this. Changing the defaults behind the back silently and throwing errors when any of those settings are set can certainly brick your nextcloud mid upgrade, especially in the future with maybe new migrations. 7.5.0 already bricked your nextcloud mid upgrade and required a hotfix to recover.

[beardhatcode]

And if we only patch the get function? That way no errors are thrown, and we know what keys are overwritten. I think we could make a test for this that just requests the a transcoded video and sees if it returns something.

[SuperSandro2000]

We could do that but I am not sure how we should communicate that to the end user.

I don't really want to write a test for this. I don't think I will get a reliable test to work. Probably the easiest to just deploy the update locally and see how the admin page behaves after a few cron runs. Without any content I think it just idles anyway and waits for more pictures to be added.

[SuperSandro2000]

Wait a second: if we patch the get functions, is that also shown in the admin settings? That would be fine for me. Let me try that later.

[beardhatcode]

Wait a second: if we patch the get functions, is that also shown in the admin settings? That would be fine for me. Let me try that later.

Yes, it is shown in the admin interface. If we do not also patch the set, the user might be able to update it but uppon refresh the nix values will be there again

[beardhatcode]

This is what I get using the defaults with my patch (and after enabling transcoding, which is off by default)

Memories - Administration settings - Nextcloud.pdf

[beardhatcode]

Downside of just patching the get and not the set is that bad values are still stored in the DB:

So if we patch I would patch both. But it also works only patching get

[beardhatcode]

Nice with the patch file :) , but in its current for this causes a syntax error for me

[beardhatcode]

Turns out we also need to set memories.exiftool_no_local to false.

I suggest to re-add the exception such that it shows up in the logs. If the user sees the failed to set they might look there.

[beardhatcode]

Suggested change

	+ return;
	+ throw new \InvalidArgumentException("Cannot set nix-managed key: {$key}");

This way you can get it in the logs at least:

[SuperSandro2000]

I really do not want to do that and fill the log app with more logs that are not actionable. I already have way to much noise in there which just exists.

We already have some options which are fixed by nextcloud and reverted on restart. We maybe need to write that somewhere down.

Also when just changing the setting in the admin UI there is no feedback that the request failed. It is logged in the browser console and in the log app.

[beardhatcode]

There is a small popup

I don't thing the popup will show if you just do return. And it only logs a message when you do an edit in the settings, which is almost never.