@teh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @bluescreen303 and @abbradar to be potential reviewers.

Maybe this can be made conditional on the length of the string? Otherwise it's going to add a huge number of files to the system closure.

@edolstra - yea see my comment on merge. My main worry was having a less-tested path around. Looking at my servers this will add ~200 files + symlinks. If you still prefer the conditional I'll add that.

Yeah, I'd prefer to avoid the overhead for the common case.

@edolstra - Please have a look. I think we also had a bug in that the entire line musn't be longer than 2048, and AFAICT we only checked the right hand side of Environment=.

Now that I think about it I'm not even sure the assumption holds that EnvironmentFile is allowed to have longer lines. I'll have a look at the systemd code.

https://github.com/systemd/systemd/blob/b965427b59a6912a46e1cdb26b732d3ea396b332/src/basic/fileio.c#L302 - doesn't look like there's a limit so the spillover solution should work.

Off-topic: The systemd parsing code looks quite scary, I really wish it wasn't c ...

Making rounds in the older PRs, this seems a worthwhile addition, what's left to do, if anything, before reviewing and then merging?

I ran the added test:

~/.../nixos/tests $ nix-build systemd-spill-to-environmentfile.nix

And everything ran fine, while running the same test on nixpkgs-unstable (without the fix from this patch) fails as expected

~/.../nixos/tests $ nix-build systemd-spill-to-environmentfile.nix
error: The value of the environment variable ‘LONGVAR’ in systemd service ‘long-env-unit.service’ is too long.
(use '--show-trace' to show detailed location information)

@teh this is super useful... are you happy with this being merged as is? I don't notice any obvious problems so after a rebase and whatnot LGTM 👍

@aanderse it seem the max line length is now ~1MiB but when I tried it failed so I did a rebase with 2048.

If the max length has been increased and it doesn't seem to be working we should probably investigate that. I would say this PR is no longer required but instead an investigation into why you are having issues and see if we can reproduce? What is your test case?

I replaced the spillover check with 1048576 + and repeated the 8-char longvar 131072 times. That caused systemd to complain.

I agree that with 1MB line length this is much less acute. My getconf ARG_MAX is 2MiB so it's still a problem but more theoretical at this point. Happy to close.

Thanks @teh!