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

CreateOrMergeNixConfig: fix expected nix.conf mode to be 644, not 664 #1316

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Nov 25, 2024

We had this bug in various places in the past, but I thought I fixed them all. No clue how it showed up again...

Description
Checklist
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

@cole-h cole-h enabled auto-merge (squash) November 25, 2024 21:18
@cole-h cole-h disabled auto-merge November 25, 2024 22:15
@cole-h
Copy link
Member Author

cole-h commented Nov 25, 2024

@cole-h cole-h marked this pull request as draft November 25, 2024 22:26
@cole-h
Copy link
Member Author

cole-h commented Dec 9, 2024

We think it should probably be 644 anyways. We'll remove the error when it differs and just unconditionally chmod it to what we want if we run into that when installing / curing.

@cole-h cole-h marked this pull request as ready for review December 9, 2024 19:07
@cole-h cole-h added this to the 0.32.2 milestone Dec 9, 2024
It has apparently been like this since the start[1], but that was
probably a typo. Let's stop perpetuating it.

[1]: https://github.com/NixOS/nix/blob/121020fc508b5731209cc158986f1e78fb99ed9f/scripts/install-multi-user.sh#L967
When we write out the new or merged config, we'll set it to our expected
mode anyways, so there's no need to error if it's different.
@cole-h cole-h force-pushed the fixup-nix-conf-mode branch from b12d6f9 to 208bfd3 Compare December 9, 2024 19:11
@cole-h cole-h enabled auto-merge (squash) December 9, 2024 19:11
@cole-h cole-h merged commit 82a6444 into main Dec 9, 2024
22 checks passed
@cole-h cole-h deleted the fixup-nix-conf-mode branch December 9, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants