nextcloud: add recognize app #333545
nextcloud: add recognize app #333545
Conversation
beardhatcode
force-pushed
the
feat/ncapps-recognise
branch
2 times, most recently
from
69a4a79
to
2c18298
Compare
beardhatcode
force-pushed
the
feat/ncapps-recognise
branch
from
2c18298
to
d37b446
Compare
|
Related PR: #211631 |
| let packages = self: | ||
| let | ||
| apps = lib.importJSON (../servers/nextcloud/packages/. + (builtins.toPath "${ncVersion}.json")); |
There was a problem hiding this comment.
| apps = lib.importJSON (../servers/nextcloud/packages/. + (builtins.toPath "${ncVersion}.json")); | |
| apps = lib.importJSON (../servers/nextcloud/packages/ + "${ncVersion}.json"); |
IIRC
| fetchurl, | ||
| lib, | ||
| nodejs, | ||
| ... |
| tar -xzpf "${builtins.elemAt srcs 0}" recognize; | ||
| tar -xzpf "${builtins.elemAt srcs 1}" recognize-${version}/models; | ||
| mv recognize-${version}/models recognize | ||
| sed -i "/'node_binary'/s:'""':'${nodejs}/bin/node':" recognize/lib/Service/SettingsService.php |
There was a problem hiding this comment.
Please use substituteInPlace
| meta = with lib; { | ||
| license = licenses.agpl3Only; | ||
| maintainers = with maintainers; [ beardhatcode ]; | ||
| longDescription = '' |
There was a problem hiding this comment.
We always must add a description
There was a problem hiding this comment.
This means we don't need something like https://github.com/NuschtOS/nixos-modules/blob/da5286bc062adee0e0aaf2bd3b784b477c623422/modules/nextcloud.nix#L172-L183 but still https://github.com/NuschtOS/nixos-modules/blob/da5286bc062adee0e0aaf2bd3b784b477c623422/modules/nextcloud.nix#L210-L217 ?
To be clear: I'm very, very 👎 on doing any build steps at runtime.
I expect the JS code to be built correctly and to have the settings patched by this package to point to other Nix packages (I.e. node, ffmpeg etc.). I looked through those super-questionable[0] "migration" "steps"[1] and I really don't want to rely on any of that.
[0] to say the least
[1] https://github.com/nextcloud/recognize/blob/main/lib/Migration/InstallDeps.php
| @@ -23,6 +23,9 @@ let packages = self: | |||
| inherit (data) url hash description homepage; | |||
| }) {}; | |||
|
|
|||
|
|
|||
| recognize = (pkgs.callPackage ./adapted/nextcloud-app-recognize.nix { }); | |||
|
|
|||
| } // lib.mapAttrs (type: pkgs: | |||
| lib.makeExtensible (_: lib.mapAttrs (pname: data: self.mkNextcloudDerivation { inherit pname; inherit data; }) pkgs)) | |||
| generatedJson; | |||
There was a problem hiding this comment.
Right below the line here there's an extend. Would it be an option to add recognize to the JSON and do the necessary overrides there?
Would have the nice side-efffect that it's updated along with all other apps.
See also #328634 for related changes.
There was a problem hiding this comment.
The problem is that the package in the apps.json of nextcloud expects that it is in a writable filesystem. And then at first run it downloads some more stuff, the 2 sources need to be aligned, therefore I would keep it out of the apps.json (also making it clear that it is a special case app).
There was a problem hiding this comment.
We should download those files at build time if possible.
We can verify if the right version was downloaded with grep or so.
I was planning to wrap the |
|
I'm also going to add a third party app to the package set #328634 |
I don't use declerative nextcloud apps, so I must do it like this. So we want to patch the app before in nextcloudApps. |
That's OK, my comment was strictly about what I'd like to see (or not see) in nixpkgs.
Most likely yes. |
Description of changes
Alters the way nextcloud28Packages etc is defined to allow adding custom packages.
In this case to add the recognize app which downloads the ML models at runtime (which is not compatible with the read only filesystem). Additionally, I also patch in the version of nodejs (in a very dirty way, for now). I tested it and it says
and
but also
This is mostly a request for comments on of how a thing like this can be done. So, @Ma27, @dotlambda, @onny, and @pyrox0 (the last 4 editors of the Nextcloud packages folder), what do you think?
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.