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.

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

nixos/tmp: Fix format of /tmp mount options #157512

Merged
merged 1 commit into from Feb 17, 2022

Conversation

r-vdp
Copy link
Contributor

@r-vdp r-vdp commented Jan 31, 2022

Motivation for this change

The mount options need to be passed as a comma-separated list of options so that they end up one a single Options line in the resulting mount unit.
The current code passed the options as a list, resulting in several Options lines in the mount unit, all but the first of these were ignored by systemd however.
This behaviour is not clearly defined in the systemd man page.

This change might require a mention in the release notes as it will cause mount options to be applied where before they weren't.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

The mount options need to be passed as a comma-separated list of options so that they
end up one a single Options line in the resulting mount unit.
The current code passed the options as a list, resulting in several Options lines in
the mount unit, all but the first of these were ignored by systemd however.
This behaviour is not clearly defined in the systemd man page.
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

LGTM.

Though I wonder if we should better address this (also) on a more general level by flattening the list even if anyone (including user config) specifies a list in similar NixOS options.

@vcunat vcunat merged commit 9d09daa into NixOS:master Feb 17, 2022
@r-vdp r-vdp deleted the fix_tmp_mount_options branch February 17, 2022 16:09
@ajs124
Copy link
Member

ajs124 commented Feb 17, 2022

This behaviour is not clearly defined in the systemd man page.

Has systemd upstream been made aware of this? IME they tend to fix documentation errors relatively fast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants