Skip to content

postfix: Add optional LDAP support, allow changing package in service #26420

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 4, 2017
Merged

postfix: Add optional LDAP support, allow changing package in service #26420

merged 1 commit into from
Jul 4, 2017
+7 −3

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Jun 6, 2017

Motivation for this change

I'm currently trying to set up a mail server (well, just a forwarder) with LDAP and Postfix. To this end, I've had to add support to LDAP to the postfix package, and allow changing the package used by the postfix service.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@roblabla
postfix: Add optional LDAP support

Verified

This commit was signed with the committer’s verified signature.
edolstra Eelco Dolstra
GPG key ID: 8170B4726D7198DE
Verified
Learn about vigilant mode
f109405
fpletz
fpletz approved these changes Jun 7, 2017
View reviewed changes
Copy link
Member

@fpletz fpletz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, but note that the maintainer @peti is opposed to adding package options: #14101

@fpletz fpletz requested a review from peti June 7, 2017 06:10
@roblabla
Copy link
Contributor Author

roblabla commented Jun 7, 2017 •
edited
Loading

Well, for reference, peti suggests using something like

  nixpkgs.config.packageOverrides = pkgs: { postfix = pkgs.postfix.override { withPgSQL = true; }; };

I'm not against it, but I feel like it's inconsistent with the way a lot of services are packaged (From those I use, nginx, redis and xonsh have a package attribute, and a bunch of other package have it too). IMO, we should either mandate every service to have the package attribute, or remove all the existing one, and have an officially mandated way to replace the package used by a service.

peti
peti requested changes Jun 11, 2017
View reviewed changes
Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

IMHO, the package attribute is redundant and doesn't add any functionality that cannot be achieved without it. Defining a package makes sense for some packages who have many (potentially incompatible) versions in Nixpkgs, like PostgreSQL, but Postfix doesn't strike me as a tool that falls into this category.

@roblabla
Copy link
Contributor Author

roblabla commented Jul 3, 2017

@peti fixed it. Sorry for the long wait ^^.

@peti peti merged commit edb5876 into NixOS:master Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peti peti

@fpletz fpletz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@roblabla @peti @fpletz