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.

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

[17.09] dovecot mailbox quoteing #34760

Merged
merged 3 commits into from Feb 9, 2018
Merged

[17.09] dovecot mailbox quoteing #34760

merged 3 commits into from Feb 9, 2018

Conversation

andir
Copy link
Member

@andir andir commented Feb 9, 2018

Motivation for this change

This is an attempt to properly backport #34759 to 17.09.

/cc @dotlambda

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
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • 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.

Ryan Trinkle and others added 3 commits February 9, 2018 13:17
@andir
nixpkgs.lib: Add escapeNixString
2c5e616 
(cherry picked from commit 6281eb1)
@andir
nixpkgs.lib: Add types.strMatching
5ce2352 
(cherry picked from commit 83048c4)
@andir
nixos/dovecot: no " in mailbox.name
5b32006 
(cherry picked from commit 6ceece6)
@dotlambda
Copy link
Member

Looks good to me. But there's no need for escapeNixString.
I don't know when builtins.match became available in Nix and if 17.09 is guaranteed to work with certain versions of Nix.

@andir
Copy link
Member Author

andir commented Feb 9, 2018

escapedNixString is required here: https://github.com/NixOS/nixpkgs/pull/34760/files#diff-4ef33a7568e82d463e872a450a6c2b7cR125

While we could technically change that this makes more sense and is in line with master.

@dotlambda
Copy link
Member

Oh, you're right.

@dotlambda
Copy link
Member

builtins.match seems to have been around since Nix 1.8: NixOS/nix@976df48
So this shouldn't be a problem.

@andir
Copy link
Member Author

andir commented Feb 9, 2018

Thanks for reviewing! @dotlambda

@andir andir merged commit 1b83e43 into NixOS:release-17.09 Feb 9, 2018
@andir andir deleted the 17.09/dovecot-mailbox-quoteing branch February 9, 2018 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andir @dotlambda @GrahamcOfBorg