nixos/anki-sync-server: init

[telotortium]

Description of changes

Provide a NixOS module for the built-in Anki Sync Server included in recent versions of Anki. This supersedes the ankisyncd module, but we should keep that for now because ankisyncd supports older versions of Anki clients than this module.

Things done

• 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:
  • 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/)
• 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.

[telotortium]

@martinetd as maintainer of ankisyncd.

[de11n]

I don't know anything about Anki so I just have random stylistic comments.

[telotortium]

@de11n in the manual test, why isn't my introduced option services.anki-sync-server.enable producing a correct reference #opt-services.anki-sync-server.enable?

[martinetd]

Thanks for your work! Everything about packaging anki has been a pain to me, so anyone working on this is greatly appreciated.
I've got a few high level questions first, didn't look at the module itself yet; I'll take a bit of time for that this weekend.

[de11n]

Much better! Thanks! I'll let others take it from here who actually know what Anki does. 😄

[martinetd]

I've played with this a bit, rebased and added a few commits in https://github.com/martinetd/nixpkgs/commits/anki-sync-server-nixos

I didn't have anywhere to cmment individually, but this is missing adding the module to nixos/modules/module-list.nix ( martinetd@ccfbafe ); which makes this hard to use :) Took me a while to understand why test didn't find services.anki-sync-server !

I've added a nixosTests and switched to the pending anki-sync-server binary; not quite sure what to do order-wise here wrt this PR, but it's identical to anki --syncserver so this is probably fine to do later.
(We might want to change services.anki-sync-server.package in favor of something like server_command that'd be either "${pkgs.anki}/bin/anki --syncserver" or "${pkgs.anki-sync-server}/bin/anki-sync-server" ...)

The added test needs the 23.10beta6 update because anki's internal changed a tiny little bit (at least col.sync_collection(login, False) needs to be changed to col.sync_collection(login) for 2.1.66); I think rest would work)
The tests need updating for 23.10 so I've split the test commit and its update as we probably don't want to wait for anki update (the anki-sync-server package looks like it'll get in fast enough to use)

Last would be to really decide how we want the modules to evolve.
Keeping both services.ankisyncd and services.anki-sync-server might seem good at first but data migration will be a pain later because we'll have users with data in either directory: I'd be in favor of replacing services.ankisyncd with this, keeping just the data path working as said last week.

WDYT?

[telotortium]

@martinetd sorry for not getting back earlier - I didn't realize you had committed so many changes on your branch. I've pushed your commits to this PR.

I am going to have to rebase to master to fix some merge conflicts in the docs.

[martinetd]

(Last review before you update the package -- as far as I'm concerned, the new module is good to go, just these small details + anki update left)

[telotortium]

@martinetd Can we drop the initial commit with 2.1.66 and directly start at 23.10 (ideally 23.10.1)? I presume so

[martinetd]

@martinetd Can we drop the initial commit with 2.1.66 and directly start at 23.10 (ideally 23.10.1)? I presume so

Sure feel free to rewrite history, but the commit should be separate as it updates anki too, not just anki-sync-server.
So I guess you can reoder the commits and squash the test fix, but that's about it?
I have no hard preference, feel free to do as you like :)

[telotortium]

@martinetd Can we drop the initial commit with 2.1.66 and directly start at 23.10 (ideally 23.10.1)? I presume so

Sure feel free to rewrite history, but the commit should be separate as it updates anki too, not just anki-sync-server. So I guess you can reoder the commits and squash the test fix, but that's about it? I have no hard preference, feel free to do as you like :)

I started with 2.1.66 and make a separate commit to update to 23.10.1.

[telotortium]

@de11n @martinetd can one of you merge?

[paveloom]

I would suggest making this dependent on #264796 and marking as a draft until that PR is merged.

[martinetd]

I would suggest making this dependent on #264796 and marking as a draft until that PR is merged.

It's been open for a while but I guess it's likely the other PR will get in first...
We don't actually really care about which version gets in except for the test that breaks; we could just remove the version update and test addition for now and add it in a later PR @telotortium ?
That way this PR won't need to wait on anything.

And I do not have merge rights, sorry. Once the release note comment is fixed up we can bring it up in https://discourse.nixos.org/t/prs-already-reviewed/2617 and someone should take a look.

[martinetd]

(continuing other outdated thread here)
happy to make the modification in the ankisyncd module later but this is just wrong as clients supported are the same, let's just write will be deprecated or something.

[wegank]

This should probably be done in a PR deprecating the module, I think.

[martinetd]

Sure, I see you removed the mention about older clients -- this is fine with me

[martinetd]

@wegank merged both the anki update and the anki-sync-server package addition just now (Thanks!!)

I'll rebase this and fix the release notes wording tonight if you don't have time before me; shouldn't be much longer :)

[wegank]

@ofborg build anki.passthru.tests

[telotortium]

@wegank can we merge?