lib.generators.toPlist: escape XML syntax in strings & keys

[linnnus]

Before this patch, code like this would break generate invalid XML.

lib.generators.toPlist {} "ab<cd"

That's obviously bad, since the call to toPlist often happens through indirection, as is the case in e.g. the nix-darwin project.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• 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/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

---

Add a 👍 reaction to pull requests you find important.

[linnnus]

Conceivably, someone could be relying on the current behavior to do some truly unholy stuff, so I guess this is technically a breaking change. In that case, what changelog should I update?

[emilazy]

This is clearly an improvement, and would be nice for nix-darwin. However I’m a bit worried about the Hyrum’s law implications here, since it is likely that people are already doing manual escaping to work around this in the wild. It may have to be an option, or else perhaps renaming the function and gradually deprecating the old one.

[linnnus]

Nice catch! I hadn't thought about that. An initial glance at public uses of generators.toPlist does show at least one repo does manual escaping, though most don't.

If it were up to me it would just be a breaking change in the next version, but to be honest I'm not used to working on projects this large, so I guess that's up to you!

[emilazy]

In an ideal world we’d do that, but lib specifically is rather conservative :) My suggestion is to change the first argument to { escape ? false }, and then we can use that in nix-darwin and Home Manager, and we can add a warning when !escape saying that it will be removed. Then after a release cycle we could toggle it on by default, and remove the flag entirely after branch‐off, say. But that’s a long period of work so I can understand not wanting to take it up :( Still, adding the option with a TODO at least means that someone else can complete the transition if they find it.

[linnnus]

I have made the changes you suggested. Escaping is now disabled by default :^)

[ambroisie]

Could you squash your PR please?

[linnnus]

Should be done know?

[ambroisie]

Thanks.

Will let our library owners chime in now.

[linnnus]

Sooo... can we merge this now, or which "library owners" should I reach out to? :^)

[ambroisie]

@infinisil @Profpatsch friendly ping.

[infinisil]

This is looking almost perfect, just a minor improvement to automate the deprecation a bit more :)

[infinisil]

Thanks, looking good!