dovecot: add lua support #109551
dovecot: add lua support #109551
Conversation
| @@ -17639,7 +17639,9 @@ in | |||
|
|
|||
| dodgy = with python3Packages; toPythonApplication dodgy; | |||
|
|
|||
| dovecot = callPackage ../servers/mail/dovecot { }; | |||
| dovecot = callPackage ../servers/mail/dovecot { | |||
| lua = lua5_4; | |||
There was a problem hiding this comment.
This is not necessary, but it builds with it and our lua defaults to 5.2 for some reason, which has been EOL since 2015.
There was a problem hiding this comment.
Maybe we should update the default lua version.
| "--with-icu" | ||
| "--with-lz4" | ||
| "--with-ssl=openssl" | ||
| "--with-zlib" |
There was a problem hiding this comment.
I reordered these, because they were quite random and ldap was enabled twice.
| @@ -12,13 +13,15 @@ stdenv.mkDerivation rec { | |||
| pname = "dovecot"; | |||
| version = "2.3.13"; | |||
|
|
|||
| nativeBuildInputs = [ perl pkgconfig ]; | |||
| # autoreconfHook can be removed as soon as the patch to fix the lua build below is removed | |||
| nativeBuildInputs = [ autoreconfHook perl pkgconfig ]; | |||
There was a problem hiding this comment.
| nativeBuildInputs = [ autoreconfHook perl pkgconfig ]; | |
| nativeBuildInputs = [ autoreconfHook perl pkg-config ]; |
| , bzip2, zlib, lz4, inotify-tools, pam, libcap | ||
| , clucene_core_2, icu, openldap, libsodium, libstemmer, cyrus_sasl | ||
| , nixosTests | ||
| # Auth modules | ||
| , withLua ? true, lua, autoreconfHook |
There was a problem hiding this comment.
I am not sure if we want to enable it by default. Do you think this would be something everyone would need or more of a niche feature?
There was a problem hiding this comment.
Because closure size and security
There was a problem hiding this comment.
This is dovecot. If you don't configure a lua userdb or passdb, this code will never run. Are you running this anywhere?
There was a problem hiding this comment.
Yes, dovecot is my imap server. But I see that you have commit access so I assume you're going to merge this anyway.
There was a problem hiding this comment.
I won't, I've decided that I'm just going to deploy this in my own system and not bother.
There was a problem hiding this comment.
I don't think that answering a simple question with a thumbs down is how we want to communicate here. If all other distros enable this by default then we can just enable it without any flag. If none of the distros enable it by default then we need to think about it a bit more. If it does not decrease security or increase closure size by that much we can also enable it by default and maybe add a flag to disable it. If it is a feature you definitely want to disable by default then a by default disabled flag is required.
I just think it is a good idea to maybe talk two sentences about such a flag in a program like dovecot which might impact a lot of people before adding a default enabled flag which might or might not cause a big change for all of them. If we can't do that then I don't think I work as a reviewer for your PRs.
There was a problem hiding this comment.
Debian splits Lua authentication into an extra package (https://packages.debian.org/buster/dovecot-auth-lua) and OpenSUSE doesn't seem to have it at all (https://build.opensuse.org/package/show/openSUSE:Factory/dovecot).
There was a problem hiding this comment.
@SuperSandro2000 "If we can't do that then I don't think I work as a reviewer for your PRs." amen to that. You really don't. Why do you think I blocked you? Because I "value" your "insights". But as I said, I won't push for this anymore. Or much else for that matter, because why would I? So yeah, do all of us a favor and don't touch my PRs anymore.
| @@ -1,8 +1,9 @@ | |||
| { stdenv, lib, fetchurl, perl, pkgconfig, systemd, openssl | |||
| { stdenv, lib, fetchurl, fetchpatch, perl, pkgconfig, systemd, openssl | |||
There was a problem hiding this comment.
| { stdenv, lib, fetchurl, fetchpatch, perl, pkgconfig, systemd, openssl | |
| { stdenv, lib, fetchurl, fetchpatch, perl, pkg-config, systemd, openssl |
| , bzip2, zlib, lz4, inotify-tools, pam, libcap | ||
| , clucene_core_2, icu, openldap, libsodium, libstemmer, cyrus_sasl | ||
| , nixosTests | ||
| # Auth modules | ||
| , withLua ? true, lua, autoreconfHook |
Motivation for this change
A feature I want/need in my e-mail setup needs this.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)