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

nix: implement flake #1869

Merged
merged 1 commit into from
Dec 1, 2022
Merged

nix: implement flake #1869

merged 1 commit into from
Dec 1, 2022

Conversation

VTimofeenko
Copy link
Contributor

This commit introduces the ability to install Docspell from a Nix flake. There are no major changes to the logic of the previous modules outside of organizing them in a flake and adding a simple check script.

@@ -0,0 +1,85 @@
{ config, pkgs, ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Diffs from original file (diff {..,checks}/configuration-test.nix)

3d2
<   docspell = import ./release.nix;
15d13
<   imports = docspell.modules ++ [ ./solr.nix ];
# now handled within the flake when constructing checks
26,36d23
<   nixpkgs = {
<     config = {
<       packageoverrides = pkgs:
<         let
<           callpackage = pkgs.lib.callpackagewith(custom // pkgs);
<           custom = {
<             docspell = callpackage docspell.currentpkg {};
<           };
<         in custom;
<     };
<   };
# no longer necessary, the packages are available in flake overlay output
60c47,48
<       { enabled = true;
---
>       {
>         enabled = true;
76c64,65
<     [ pkgs.docspell.server
---
>     [
>       pkgs.docspell.server
91c80
<     firewall.allowedtcpports = [7880];
---
>     firewall.allowedtcpports = [ 7880 ];
# minor nixpkgs-fmt

@@ -0,0 +1,7 @@
{ ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imports for the flake check happen here

@@ -0,0 +1,35 @@
{ config, pkgs, ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from original into the flake to be self-contained

@@ -0,0 +1,3 @@
with subtest("services are up"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Primitive test script that waits for systemd units inside the test VM

forAllSystems = nixpkgs.lib.genAttrs supportedSystems;
nixpkgsFor = forAllSystems (system: import nixpkgs { inherit system; });
# Version config
cfg = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Taken from the original pkg.nix, just for the latest version for brevity's sake.

If the original implementation in nix/ is kept - the flake could import this attrset from pkg.nix

@@ -0,0 +1,1728 @@
overlay: { config, lib, pkgs, ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key change: module by itself takes an overlay to later reference the package without a a call to callPackage.

The rest of changes to this file (diff ../module-joex.nix modules/joex.nix) are mostly nixpkgs-fmt.

@@ -0,0 +1,877 @@
overlay: { config, lib, pkgs, ... }:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key change: module by itself takes an overlay to later reference the package without a a call to callPackage.

The rest of changes to this file (diff ../module-server.nix modules/server.nix) are mostly nixpkgs-fmt.

@VTimofeenko
Copy link
Contributor Author

As discussed yesterday in Gitter - opening a PR to add the ability to install Docspell through flakes. I have not included any material changes to the logic that is implemented in nix - I will file those PRs later so that they would be easier to review.

In my environment the flake successfully installs and runs Docspell on an aarch64-linux machine and passes checks on a x86_64-linux machine.

Since some of the diffs are quite long - I have added comments to the first lines to summarize the changes.

Question: should I include the instructions to use flake in the docs?

Copy link
Owner

@eikek eikek left a comment

Choose a reason for hiding this comment

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

Thank you so much! 💯

Big thanks also for the comments, it helped to look through it. I think we can now remove the "old" files right? Just to reduce maintenance burden a little. Maybe an example on how to use it would be nice to add to the readme or docs eventually. wdyt?

Edit: I'm fine with not defining packages for every version. It is easy enough to go some commits back if needed

};


# nixosModules = {
Copy link
Owner

Choose a reason for hiding this comment

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

is this some example or maybe a leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leftover, will kill this part

@VTimofeenko
Copy link
Contributor Author

You're most welcome :) I have been a user of docspell for probably a couple of years now and glad to be able to give something back.

I think we can now remove the "old" files right? Just to reduce maintenance burden a little.

From technical perspective - yes, old files can be removed.

Maybe an example on how to use it would be nice to add to the readme or docs eventually. wdyt?

Sure, can do.

I'm fine with not defining packages for every version. It is easy enough to go some commits back if needed

One way to tackle this - probably in a future PR - is to:

  1. Implement building from source (also would close For nix, build from source? #652)
  2. Move flake.nix to the root of the project
  3. Replace all references to the package source in the flake to self.

That way a user would be able to add to their inputs something like:

inputs.docspell.url="github:eikek/docspell?ref=0.39.0`;

and nix would take care of the rest.

All in all what I will change in this commit:

  1. Remove the obsolete comment in flake.nix as per discussion
  2. Remove the code in nix/
  3. Merge older tags from pkg.nix into the attrset flake.nix (for now)
  4. Move the flake to nix/
  5. Describe how the flake can be used in website/site/content/docs/install/nix.md

@eikek
Copy link
Owner

eikek commented Nov 29, 2022

Thank you, that is really great! For me it is fine to just go for flakes and remove all other redundant code. By "not defining packages for every version" I merely meant to only define the current version in the nix files as you did it. The huge list of previous versions can be removed in my view (so just as you did).

That way a user would be able to add to their inputs something like:

inputs.docspell.url="github:eikek/docspell?ref=0.39.0`;

Yes that would be nice, of course. I sense some effort here, but I never really built an sbt project via nix. Perhaps good to keep for later :)

I can hit "merge" right now if you want or later, just ping me when you are ok. There is no hurry :)

This commit introduces the ability to install Docspell from a Nix flake.
There are no major changes to the logic of the previous modules outside of
organizing them in a flake and adding a simple check script.
@VTimofeenko
Copy link
Contributor Author

I have implemented the aforementioned changes

A couple of points:

  1. Documentation mentioned consumedir module - but I could not find a definition for one. Did I miss it? I removed it from doc for now

  2. I have pinned nixpkgs version to nixos-22.11 as it just came out - turns out two packages are currently marked as insecure:

    • solr - directly
    • wkhtmltopdf - indirectly, since it depends on qtwebengine which is deemed insecure (see here)

    I have added them to permitInsecurePackages in tests.

@eikek
Copy link
Owner

eikek commented Dec 1, 2022

Thank you so much!
Regarding

  1. oh right - this has been moved a long time ago into the dsc project :-)
  2. I think that's good to add them to permitted packages - thank you.

@eikek eikek merged commit 4014196 into eikek:master Dec 1, 2022
@VTimofeenko VTimofeenko deleted the nix-flake branch December 12, 2022 16:03
@eikek eikek added this to the Docspell 0.40.0 milestone Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants