-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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/seafile: init service #119719
nixos/seafile: init service #119719
Conversation
Also please create the commits according to the contributing guide. |
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.
Thanks for contributing this. I have left a few comments.
I'd also like to point out that, as you mention, upstream highly suggests not packaging this application and just running their docker image or whatever. Are you going to be able to properly maintain this in nixpkgs?
inherit stdenv lib fetchFromGitHub cmake libevent; | ||
}; | ||
oniguruma = import ./oniguruma.nix { inherit stdenv lib fetchFromGitHub cmake; }; | ||
in stdenv.mkDerivation rec { |
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.
I'm getting errors when trying some of these programs out:
~/nixpkgs ((792b8f64…))> /nix/store/kmhchjnd2ixm8n7bq23p6map8550qwig-seafile-server-8.0.4/bin/seafile-admin --help
File "/nix/store/kmhchjnd2ixm8n7bq23p6map8550qwig-seafile-server-8.0.4/bin/seafile-admin", line 12
print 'Python 3 not supported yet. Quit now'
^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print('Python 3 not supported yet. Quit now')?
And this one says nothing:
~/nixpkgs ((792b8f64…))> /nix/store/kmhchjnd2ixm8n7bq23p6map8550qwig-seafile-server-8.0.4/bin/seaf-server --help
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.
Unfortunately that reflects the state of upstream 😒
seaf-server
does not seem to respond to --help but requires several options to run. (see ExecStart of seaf-server for a working example)
As for seafile-admin
, it's part of the official distribution but also fails on python3 so I guess this is a leftover 🤷.
It's not mentioned in the documentation either. It looks like something used before setup-seafile.sh.
ff7f70c
to
3d3a35e
Compare
This is a very valid concern regarding quality and long-term support of packages in NixOS. As for my ability to maintain the package: any answer would be an empty promise 😄 Now, to be honest, I'm not a nix expert (learnt a lot thanks to your great feedback here already) and I hope this service can get enough community interest to get some help on the long-term. |
Good answers 😄 |
Or if you give me push access to your fork I guess I could fix this myself @greizgh |
Thank you @schmittlauch I've just rebased the branch 👍. (Meanwhile I'll give you access to my fork to ease future maintenance). |
It succeeds for me. Apparently this line was forgotten in PR NixOS#119719.
Glad to see this got merged! This might just be something I'm doing wrong, but any ideas as to what could have caused this? Seahub.log error
|
I just hit the same issue that @Pacman99 has hit. I am fronting everything with nginx and the following virtualhost entry:
|
@Pacman99 @qbit the issue is related to an incompatible dependency version. PR with a fix. |
@@ -6950,6 +6954,8 @@ in { | |||
|
|||
pysdl2 = callPackage ../development/python-modules/pysdl2 { }; | |||
|
|||
pysearpc = toPythonModule pkgs.libsearpc; |
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 is garbage. Remove immediately!
EDIT: Use the correct version of Python instead.
@@ -8311,6 +8317,10 @@ in { | |||
|
|||
seabreeze = callPackage ../development/python-modules/seabreeze { }; | |||
|
|||
seahub = callPackage ../development/python-modules/seahub { }; | |||
|
|||
seaserv = toPythonModule pkgs.seafile-server; |
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 is garbage. Remove immediately!
EDIT: Use the correct version of Python instead.
@@ -0,0 +1,56 @@ | |||
{ stdenv, lib, fetchFromGitHub, python3Packages, makeWrapper }: | |||
|
|||
python3Packages.buildPythonPackage rec { |
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.
Is this supposed to be used as import seahub
? If not, this should not be in pythonPackages
and should use buildPythonApplication
.
Not even the commit messages had the proper format I will open a PR reverting this. |
I did not intend to criticize the author of this PR who probably had no idea that many things went terribly wrong here but I find it completely unacceptable that the PR was merged in this state. |
@dotlambda I am a noob bystander and have no idea why the things you point out are issues. Would you mind going into a bit more detail on why this approach isn't good? |
Commit messages for Python packages start with e.g. See https://nixos.org/manual/nixpkgs/unstable/#python if you want to know more. |
Here's an example of what needs to be done: dotlambda@14ded1f |
Thank you @dotlambda for the fixes and explanation 👍 |
I was thinking about migrating from nextcloud to seafile using this service. Is this in a state where it can be used productively? I'm a bit concerned it might run into bugs or even be removed after I migrate my data and workflow to it. |
Motivation for this change
This PR adds a seafile service.
It's partly based on previous work in #15900.
Seafile's official documentation suggest to download pre-built binaries or use a docker-compose deployment based on a sub-optimal (hardcoded path, multiple services) image.
Neither options are thrilling, that's why I went the "build from source" route and diverged from official documentation.
One component which does not appear here, but is the entry-point when deploying the official seafile "distribution", is the seafile-controller binary.
It turns out it is mostly a supervisor for the different components (seafile-server, seahub, seafdav) and can be replaced by proper systemd units.
Since "pro" components are out of scope, there are not many moving parts: only seafile-server and seahub (the web UI).
Things I checked:
Most of the tests were done with the following container definition
Feedback is highly welcome!
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)