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

Integrate nix-config-parser #263

Merged
merged 53 commits into from
Mar 6, 2023
Merged

Integrate nix-config-parser #263

merged 53 commits into from
Mar 6, 2023

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Feb 22, 2023 •
edited
Loading

Description

Closes #174.
Progress towards #126.

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

TODO:

  • update documentation on the new merge stuff, since it was ripped straight from create_file
  • maybe remove buf struct member in favor of the various nix_configs members (including in logging)
  • write some tests for nix-config-parser (taken from upstream?)
  • update the darwin json plan
  • fix the installer not actually working? maybe I messed something up somewhere in the conditionals...?
  • make nix-config-parser accept Option<Path> when parsing a string -- Make path Optional in parse_nix_config_string nix-config-parser#2
  • atomic move the resulting nix.conf
  • release a 0.1.0 of nix-config-parser so we can depend on it for real
  • address any feedback
  • use actionerror::custom and module-specific error enum
  • try to preserve comments (just replace the contents of the line up to the comment char)
  • error if user edited file, or do the merging / checking at execute time instead of plan time

@cole-h cole-h self-assigned this Feb 22, 2023
Hoverbear
Hoverbear reviewed Feb 23, 2023
View reviewed changes
src/action/base/create_or_merge_nix_config.rs Outdated
.metadata()
.map_err(|e| ActionError::GettingMetadata(this.path.clone(), e))?;

if let Some(mode) = mode {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is our third occurrence of this code so we should consider extracting it in a future PR....

Hoverbear
Hoverbear reviewed Feb 23, 2023
View reviewed changes
src/action/mod.rs Outdated Show resolved Hide resolved
Hoverbear
Hoverbear reviewed Feb 23, 2023
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@cole-h
use nix-config-parser optional<path> branch
334d918
@cole-h
checkpoint: update docs and action info
2133547
@cole-h
Merge remote-tracking branch 'origin/main' into integrate-config-parser
017b32d
@cole-h
checkpoint: remove unused arguments
b4898f3 
We never used user or group, so drop the related complexity. Easy to
resurrect if we ever need to.
@cole-h
checkpoint: remove buf member
60489a5 
It's basically recorded in the merged_nix_config anyways.
@cole-h
checkpoint: update docs
ab839be
@cole-h
bump nix-config-parser
020528e
@cole-h
checkpoint: create temp file and atomically move
2876863
@cole-h
fixup: order imports into std, external, crate
71ed042
@cole-h
fixup: remove unnecessary type ascription
db15309
@cole-h
fixup: create_file -> create_or_merge_nix_config
b7805e5
@cole-h
fixup: set permissions after we're donwe writing
3fd8d64
@cole-h
checkpoint: move errors to local enum
44874a1
@cole-h
edge case cleanup and document why that one expect() is safe
35c9751 
also rename the const to more accurately reflect that it's a `nix.conf`
comment character, not a comment character for the Nix language (despite
being true for both cases).
@cole-h
better variable names
aa73b64
@cole-h
fixup: drop println
c704a79
Hoverbear
Hoverbear reviewed Mar 3, 2023
View reviewed changes
src/action/base/create_or_merge_nix_config.rs
"If {} already exists, we will attempt to merge the current settings with our settings; \
otherwise, it will be created with only our settings",
self.path.display()
)],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this message mention which settings it will change? (I think we have them here anyways)

Hoverbear
Hoverbear reviewed Mar 3, 2023
View reviewed changes
src/action/base/create_or_merge_nix_config.rs Outdated
vec![ActionDescription::new(
self.tracing_synopsis(),
vec![format!(
"If {} already exists, we will attempt to merge the current settings with our settings; \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure the wording/styling of using "our" here matches the current plan explanation format:

* Create directory `/nix`
* Fetch `https://releases.nixos.org/nix/nix-2.13.2/nix-2.13.2-x86_64-linux.tar.xz` to `/nix/temp-install-dir`
* Create build users (UID 30000-30032) and group (GID 30000)
  The Nix daemon requires system users (and a group they share) which it can act as in order to build
  Create group `nixbld` (GID 30000)
  Create user `nixbld0` (UID 30000) in group `nixbld` (GID 30000)
  Create user `nixbld1` (UID 30001) in group `nixbld` (GID 30000)
  Create user `nixbld2` (UID 30002) in group `nixbld` (GID 30000)
  Create user `nixbld3` (UID 30003) in group `nixbld` (GID 30000)
  Create user `nixbld4` (UID 30004) in group `nixbld` (GID 30000)
  Create user `nixbld5` (UID 30005) in group `nixbld` (GID 30000)
  Create user `nixbld6` (UID 30006) in group `nixbld` (GID 30000)
  Create user `nixbld7` (UID 30007) in group `nixbld` (GID 30000)
  Create user `nixbld8` (UID 30008) in group `nixbld` (GID 30000)
  Create user `nixbld9` (UID 30009) in group `nixbld` (GID 30000)
  Create user `nixbld10` (UID 30010) in group `nixbld` (GID 30000)
  Create user `nixbld11` (UID 30011) in group `nixbld` (GID 30000)
  Create user `nixbld12` (UID 30012) in group `nixbld` (GID 30000)
  Create user `nixbld13` (UID 30013) in group `nixbld` (GID 30000)
  Create user `nixbld14` (UID 30014) in group `nixbld` (GID 30000)
  Create user `nixbld15` (UID 30015) in group `nixbld` (GID 30000)
  Create user `nixbld16` (UID 30016) in group `nixbld` (GID 30000)
  Create user `nixbld17` (UID 30017) in group `nixbld` (GID 30000)
  Create user `nixbld18` (UID 30018) in group `nixbld` (GID 30000)
  Create user `nixbld19` (UID 30019) in group `nixbld` (GID 30000)
  Create user `nixbld20` (UID 30020) in group `nixbld` (GID 30000)
  Create user `nixbld21` (UID 30021) in group `nixbld` (GID 30000)
  Create user `nixbld22` (UID 30022) in group `nixbld` (GID 30000)
  Create user `nixbld23` (UID 30023) in group `nixbld` (GID 30000)
  Create user `nixbld24` (UID 30024) in group `nixbld` (GID 30000)
  Create user `nixbld25` (UID 30025) in group `nixbld` (GID 30000)
  Create user `nixbld26` (UID 30026) in group `nixbld` (GID 30000)
  Create user `nixbld27` (UID 30027) in group `nixbld` (GID 30000)
  Create user `nixbld28` (UID 30028) in group `nixbld` (GID 30000)
  Create user `nixbld29` (UID 30029) in group `nixbld` (GID 30000)
  Create user `nixbld30` (UID 30030) in group `nixbld` (GID 30000)
  Create user `nixbld31` (UID 30031) in group `nixbld` (GID 30000)
  Add user `nixbld0` (UID 30000) to group `nixbld` (GID 30000)
  Add user `nixbld1` (UID 30001) to group `nixbld` (GID 30000)
  Add user `nixbld2` (UID 30002) to group `nixbld` (GID 30000)
  Add user `nixbld3` (UID 30003) to group `nixbld` (GID 30000)
  Add user `nixbld4` (UID 30004) to group `nixbld` (GID 30000)
  Add user `nixbld5` (UID 30005) to group `nixbld` (GID 30000)
  Add user `nixbld6` (UID 30006) to group `nixbld` (GID 30000)
  Add user `nixbld7` (UID 30007) to group `nixbld` (GID 30000)
  Add user `nixbld8` (UID 30008) to group `nixbld` (GID 30000)
  Add user `nixbld9` (UID 30009) to group `nixbld` (GID 30000)
  Add user `nixbld10` (UID 30010) to group `nixbld` (GID 30000)
  Add user `nixbld11` (UID 30011) to group `nixbld` (GID 30000)
  Add user `nixbld12` (UID 30012) to group `nixbld` (GID 30000)
  Add user `nixbld13` (UID 30013) to group `nixbld` (GID 30000)
  Add user `nixbld14` (UID 30014) to group `nixbld` (GID 30000)
  Add user `nixbld15` (UID 30015) to group `nixbld` (GID 30000)
  Add user `nixbld16` (UID 30016) to group `nixbld` (GID 30000)
  Add user `nixbld17` (UID 30017) to group `nixbld` (GID 30000)
  Add user `nixbld18` (UID 30018) to group `nixbld` (GID 30000)
  Add user `nixbld19` (UID 30019) to group `nixbld` (GID 30000)
  Add user `nixbld20` (UID 30020) to group `nixbld` (GID 30000)
  Add user `nixbld21` (UID 30021) to group `nixbld` (GID 30000)
  Add user `nixbld22` (UID 30022) to group `nixbld` (GID 30000)
  Add user `nixbld23` (UID 30023) to group `nixbld` (GID 30000)
  Add user `nixbld24` (UID 30024) to group `nixbld` (GID 30000)
  Add user `nixbld25` (UID 30025) to group `nixbld` (GID 30000)
  Add user `nixbld26` (UID 30026) to group `nixbld` (GID 30000)
  Add user `nixbld27` (UID 30027) to group `nixbld` (GID 30000)
  Add user `nixbld28` (UID 30028) to group `nixbld` (GID 30000)
  Add user `nixbld29` (UID 30029) to group `nixbld` (GID 30000)
  Add user `nixbld30` (UID 30030) to group `nixbld` (GID 30000)
  Add user `nixbld31` (UID 30031) to group `nixbld` (GID 30000)
* Create a directory tree in `/nix`
  Create directory `/nix/var`
  Create directory `/nix/var/log`
  Create directory `/nix/var/log/nix`
  Create directory `/nix/var/log/nix/drvs`
  Create directory `/nix/var/nix`
  Create directory `/nix/var/nix/db`
  Create directory `/nix/var/nix/gcroots`
  Create directory `/nix/var/nix/gcroots/per-user`
  Create directory `/nix/var/nix/profiles`
  Create directory `/nix/var/nix/profiles/per-user`
  Create directory `/nix/var/nix/temproots`
  Create directory `/nix/var/nix/userpool`
  Create directory `/nix/var/nix/daemon-socket`
* Move the downloaded Nix into `/nix`
  Nix is being downloaded to `/nix/temp-install-dir` and should be in `/nix`
* Setup the default Nix profile
* Place the Nix configuration in `/etc/nix/nix.conf`
  This file is read by the Nix daemon to set its configuration options at runtime.
* Place channel configuration at `/root/.nix-channels`
* Configure the shell profiles
  Update shell profiles to import Nix
* Configure Nix daemon related settings with systemd
  Run `systemd-tempfiles --create --prefix=/nix/var/nix`
  Run `systemctl link /nix/var/nix/profiles/default/lib/systemd/system/nix-daemon.service`
  Run `systemctl link /nix/var/nix/profiles/default/lib/systemd/system/nix-daemon.socket`
  Run `systemctl daemon-reload`
  Run `systemctl enable --now /nix/var/nix/profiles/default/lib/systemd/system/nix-daemon.socket`

Perhaps like "Update the /etc/nix/nix.conf settings $LIST_OF_SETTINGS`?"

Hoverbear
Hoverbear reviewed Mar 3, 2023
View reviewed changes
Copy link
Contributor

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

This looks good! As I mentioned in DMs with you I am slightly concerned about someone getting to the install prompt (when we have built the plan already) and having a terminal open editing their nix.conf then executing the plan and us forgetting those changes.

We discussed how we should add a simple check, or do the same planning behavior at execute time as well (checking twice, basically), with plan-time being just a validation step.

@Hoverbear Hoverbear added this to the v0.5.0 milestone Mar 3, 2023
@Hoverbear
Copy link
Contributor

I'm sorry about the merge conflict!

@cole-h
Copy link
Member Author

cole-h commented Mar 3, 2023

I'd like to move the stuff into execute as you suggested eventually, but for now this is a good first step.

@cole-h
check mergeable at plan and execute time
493a839 
This way we don't need to diff between plan time and exeucte time -- we
just accept whatever exists at execute time is what we want to modify
(if possible).
@cole-h cole-h enabled auto-merge (squash) March 6, 2023 17:19
@cole-h cole-h merged commit 2594316 into main Mar 6, 2023
@cole-h cole-h deleted the integrate-config-parser branch March 6, 2023 17:29
@lucperkins lucperkins mentioned this pull request Jun 16, 2023
Merged
This was referenced May 23, 2024
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hoverbear Hoverbear Hoverbear approved these changes

Assignees

@cole-h cole-h

Labels
None yet
Projects
None yet
Milestone
v0.5.0
Development

Successfully merging this pull request may close these issues.

Cleverness to merge extra-conf etc
2 participants
@cole-h @Hoverbear