chore: update dependencies #1677
Conversation
WalkthroughUpdate CI workflow action versions, bump multiple Rust dependencies, refresh docs and test images, broaden Darwin OS pattern matches, migrate rand API calls, adjust a few error format strings, add dead-code allowances, and update a WSL test asset. No new features or control-flow changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/action/base/create_or_insert_into_file.rs (1)
116-122: Fix incorrect error variant: use NoGroup instead of NoUser.When the group lookup fails, the error should be NoGroup, not NoUser.
Apply this diff:
- .ok_or_else(|| ActionErrorKind::NoUser(group.clone())) + .ok_or_else(|| ActionErrorKind::NoGroup(group.clone()))src/action/macos/encrypt_apfs_volume.rs (1)
165-173: Use a cryptographically secure RNG for password generation.rand::rng() is not intended for crypto. For APFS passwords, use OsRng.
Apply this diff:
- let mut rng = rand::rng(); + let mut rng = rand::rngs::OsRng; @@ - let idx = rng.random_range(0..CHARSET.len()); + let idx = rng.random_range(0..CHARSET.len());No import is required; refer to rand::rngs::OsRng directly as shown.
Also applies to: 168-171
src/action/base/add_user_to_group.rs (6)
245-248: Bug: double-wrapped error (Self::error(Self::error(...))) causes a type mismatch.The outer
Self::errorexpects anActionErrorKind, but receives anActionError. This won’t compile.Apply this diff:
- } else { - return Err(Self::error(Self::error( - ActionErrorKind::MissingAddUserToGroupCommand, - ))); - } + } else { + return Err(Self::error(ActionErrorKind::MissingAddUserToGroupCommand)); + }
141-143: Group membership parsing can miss matches due to newline/colon tokens.
split(' ')will not strip newlines and may fail to detect membership. Usesplit_whitespace()to be robust across platforms.Apply this diff:
- let output_str = String::from_utf8(output.stdout).map_err(Self::error)?; - let user_in_group = output_str.split(' ').any(|v| v == this.groupname); + let output_str = String::from_utf8(output.stdout).map_err(Self::error)?; + let user_in_group = output_str + .split_whitespace() + .any(|v| v == this.groupname);
75-83: Inconsistent pattern shape for MacOSX; align with struct pattern used elsewhere.Other modules match
OperatingSystem::MacOSX { .. }. Here it’sMacOSX(_). Aligning avoids future breakage if the variant remains struct-like in target-lexicon 0.13.x.Apply this diff:
- OperatingSystem::MacOSX(_) | OperatingSystem::Darwin(_) => { + OperatingSystem::MacOSX { .. } | OperatingSystem::Darwin(_) => {
195-223: Same MacOSX pattern inconsistency in execute().Use the same struct pattern as elsewhere.
Apply this diff:
- OperatingSystem::MacOSX(_) | OperatingSystem::Darwin(_) => { + OperatingSystem::MacOSX { .. } | OperatingSystem::Darwin(_) => {
276-289: Same MacOSX pattern inconsistency in revert().Use the struct pattern.
Apply this diff:
- OperatingSystem::MacOSX(_) | OperatingSystem::Darwin(_) => { + OperatingSystem::MacOSX { .. } | OperatingSystem::Darwin(_) => {
1-320: Replace tuple-styleOperatingSystem::MacOSX(_)with struct patterns and fix nestedSelf::error
- Change all
OperatingSystem::MacOSX(_)match arms in
- src/action/base/create_group.rs
- src/action/common/create_users_and_groups.rs
- src/action/base/add_user_to_group.rs
toOperatingSystem::MacOSX { .. }.- In src/action/base/add_user_to_group.rs (lines 245–247), remove the double-wrapped
Self::error(Self::error(...))and return
Err(Self::error(ActionErrorKind::MissingAddUserToGroupCommand))instead.src/action/base/create_directory.rs (1)
77-81: Minor: Wrong error kind on missing groupThis returns NoUser for a missing group. Prefer NoGroup for accuracy.
- .ok_or_else(|| ActionErrorKind::NoUser(group.clone())) + .ok_or_else(|| ActionErrorKind::NoGroup(group.clone()))
🧹 Nitpick comments (17)
tests/windows/test-wsl.ps1 (1)
7-17: Reuse the defined download URL.We already introduce
$url, but theInvoke-WebRequeststill hardcodes the string. Pointing it at$urlkeeps the source of truth in one place and avoids the next update forgetting one of the copies.- Invoke-WebRequest -Uri "https://cloud-images.ubuntu.com/wsl/jammy/current/ubuntu-jammy-wsl-amd64-ubuntu22.04lts.rootfs.tar.gz" -OutFile $Image + Invoke-WebRequest -Uri $url -OutFile $Imagenix/tests/container-test/default.nix (1)
9-16: Optional: Add arm64 variants for broader coverage.If ARM runners are available (as noted in the PR), consider adding ubuntu-base-24.04.3-base-arm64.tar.gz alongside amd64 with system="aarch64-linux".
Would you like a follow-up patch that adds an aarch64 entry mirroring ubuntu-v24_04?
nix/tests/vm-test/default.nix (2)
450-457: Parity suggestion: Consider adding Ubuntu 24.04 VM box.Container tests now cover 24.04; adding a generic ubuntu2404 VM box would keep VM parity (if a stable libvirt box exists).
If you want, I can draft an ubuntu-v24_04 entry with the current generic ubuntu2404 libvirt box URL/version.
459-468: Review Fedora versions against EOL cadence.Fedora 38 is near/at EOL; consider bumping to a supported release (e.g., 40/41) to avoid future breakage while keeping one older Fedora if needed.
Should I propose updated fedora40/fedora41 entries with pinned versions/hashes?
Also applies to: 469-478
.github/workflows/build-x86_64-darwin.yml (1)
16-17: Pin DS actions instead of using @main.Replace DeterminateSystems/determinate-nix-action@main and DeterminateSystems/flakehub-cache-action@main with versioned tags or SHAs (align with update.yml using determinate-nix-action@v3).
Please verify the latest stable tags for these actions and update accordingly.
.github/workflows/build-x86_64-linux.yml (1)
16-17: Avoid @main for actions; pin to tags/SHAs.Use stable tags (e.g., determinate-nix-action@v3) or commit SHAs for both DS actions.
Please confirm the appropriate tags to pin.
.github/workflows/build-aarch64-darwin.yml (1)
16-17: Pin DS actions; avoid @main.Switch to versioned tags or SHAs for determinate-nix-action and flakehub-cache-action to reduce CI drift.
Confirm latest stable tags and update.
.github/workflows/build-aarch64-linux.yml (1)
16-17: Pin DS actions instead of @main.Align with other workflows by using versioned tags (e.g., determinate-nix-action@v3) or SHAs for both actions.
Please verify and update to stable tags.
flake.nix (2)
5-8: Avoid hard-coding Nix tarball version; derive from inputs.Hard-coding 2.31.1 risks drift with inputs.nix. Prefer using nixTarballs to set NIX_TARBALL_URL so it always matches the selected input.
Example change (conceptual):
# Replace: nix_tarball_url_prefix = "https://releases.nixos.org/nix/nix-2.31.1/nix-2.31.1-"; # And later: env = sharedAttrs.env // { RUSTFLAGS = "--cfg tokio_unstable"; NIX_TARBALL_URL = nixTarballs.${stdenv.hostPlatform.system}; DETERMINATE_NIX_TARBALL_PATH = nixTarballs.${stdenv.hostPlatform.system}; DETERMINATE_NIXD_BINARY_PATH = optionalPathToDeterminateNixd stdenv.hostPlatform.system; }; # Similarly in devShells: NIX_TARBALL_URL = nixTarballs.${pkgs.stdenv.hostPlatform.system};This keeps upstream and embedded tarballs in lockstep.
Also applies to: 37-37
126-126: Nice: cargo-audit added to devShell.Optionally add cargo-deny and/or a CI check to gate PRs on advisories for better automation.
.github/workflows/update.yml (1)
19-19: Pin update-flake-lock to v27
- In .github/workflows/update.yml (line 19), replace
- uses: DeterminateSystems/update-flake-lock@main + uses: DeterminateSystems/update-flake-lock@v27No breaking changes exist between v27 and main.
src/action/base/create_or_insert_into_file.rs (1)
213-217: Prefer secure, race-free temp creation in target directory.Creating a temp path via random name can still race; consider tempfile::Builder in the parent dir to get O_EXCL semantics.
Example:
let mut builder = tempfile::Builder::new(); let tmp = builder.prefix("nix-installer-tmp.").tempfile_in(&parent_dir)?; let temp_file_path = tmp.path().to_owned(); // use tmp.reopen() or create via OpenOptions on temp_file_path; rename at the end.github/workflows/release-prs.yml (1)
79-79: Pin actions to commit SHAs for supply‑chain hardening.actions/checkout@v5 is fine, but pinning to a specific SHA is recommended per GitHub security guidance.
Example:
uses: actions/checkout@3df... # v5 SHA.github/workflows/release-tags.yml (1)
32-71: Action version bumps look good; OIDC-ready and release upload preserved.
- checkout@v5, download-artifact@v5, aws-credentials@v5, and action-gh-release@v2 usages align with defaults.
- permissions already include contents: write and id-token: write for OIDC; good.
Optional: consider pinning actions to commit SHAs for supply-chain hardening, or add a brief comment linking to release tags you trust.
src/action/base/create_or_merge_nix_config.rs (1)
301-311: Use create_new(true) or tempfile to avoid TOCTOU on temp file creation.
create(true).truncate(true)can clobber unexpectedly if a guessed path exists. Prefercreate_new(true)ortempfile::NamedTempFile::new_in(parent_dir).Apply this minimal change:
- let mut temp_file = OpenOptions::new() - .create(true) + let mut temp_file = OpenOptions::new() + .create_new(true) .truncate(true) .write(true) // If the file is created, ensure that it has harmlessOr switch to
tempfilefor race-free creation..github/workflows/release-branches.yml (1)
35-35: Explicit boolean inputs for aws-actions/configure-aws-credentials@v5 & SHA pinning
- checkout/download-artifact: verify the artifact lands where your
mv/chmodexpect.- aws-actions/configure-aws-credentials@v5: no input names changed, but boolean inputs now strictly parse only
true/false—replace any unset, empty or custom strings with explicit values.- Pin to an exact
5.x.ySHA or tag to avoid drifting to unintended minor/patch releases..github/workflows/ci.yml (1)
33-33: Confirm checkout@v5 and download-artifact@v5 behavior
- download-artifact@v5 now consistently extracts single artifacts directly into the target path (path/) for both name and ID downloads, so your existing mv/chmod steps remain valid.
- checkout@v5 upgrades to Node 24 runtime (requires GitHub Actions Runner ≥ v2.327.1) but all inputs and defaults are unchanged from v4.
- Optional: pin actions/* and DeterminateSystems/* to commit SHAs for supply-chain hardening.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.github/workflows/build-aarch64-darwin.yml(1 hunks).github/workflows/build-aarch64-linux.yml(1 hunks).github/workflows/build-x86_64-darwin.yml(1 hunks).github/workflows/build-x86_64-linux.yml(1 hunks).github/workflows/ci.yml(7 hunks).github/workflows/release-branches.yml(2 hunks).github/workflows/release-prs.yml(2 hunks).github/workflows/release-tags.yml(3 hunks).github/workflows/update.yml(1 hunks)Cargo.toml(1 hunks)README.md(3 hunks)docs/building.md(3 hunks)docs/rust-library.md(2 hunks)flake.nix(2 hunks)nix/tests/container-test/default.nix(1 hunks)nix/tests/vm-test/default.nix(2 hunks)src/action/base/add_user_to_group.rs(4 hunks)src/action/base/create_directory.rs(2 hunks)src/action/base/create_group.rs(3 hunks)src/action/base/create_or_insert_into_file.rs(1 hunks)src/action/base/create_or_merge_nix_config.rs(2 hunks)src/action/base/create_user.rs(3 hunks)src/action/base/delete_user.rs(2 hunks)src/action/common/create_users_and_groups.rs(1 hunks)src/action/common/place_nix_configuration.rs(1 hunks)src/action/macos/encrypt_apfs_volume.rs(1 hunks)src/action/macos/kickstart_launchctl_service.rs(1 hunks)src/action/mod.rs(1 hunks)src/cli/mod.rs(1 hunks)src/cli/subcommand/install/determinate.rs(1 hunks)src/cli/subcommand/repair.rs(2 hunks)src/cli/subcommand/uninstall.rs(1 hunks)src/distribution.rs(1 hunks)src/planner/macos/mod.rs(1 hunks)src/planner/mod.rs(3 hunks)src/profile/nixenv/mod.rs(1 hunks)src/profile/nixprofile/mod.rs(1 hunks)src/settings.rs(4 hunks)tests/windows/test-wsl.ps1(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/planner/mod.rs (1)
src/settings.rs (2)
default(212-257)default(390-414)
🔇 Additional comments (40)
src/profile/nixenv/mod.rs (1)
268-271: Scoped lint suppression looks good.Targeting the
outputsfield keeps the warning quiet without broadening the allowance beyond what we need.src/profile/nixprofile/mod.rs (1)
286-289: Thanks for the localized dead-code allowance.Confining the lint suppression to the field keeps the rest of the module under the usual scrutiny.
nix/tests/container-test/default.nix (1)
9-16: Verify Ubuntu base tarball URLs and SHA256 hashes in CIThe status-check script has a syntax error in its
iftest. Update and re-run it in your CI workflow. For example:#!/usr/bin/env bash set -euo pipefail file="nix/tests/container-test/default.nix" echo "Checking URLs in $file…" rg -o 'url\s*=\s*"[^"]+"' "$file" \ | sed -E 's/.*"([^"]+)".*/\1/' \ | while read -r url; do code=$(curl -s -o /dev/null -w '%{http_code}' -I -L "$url" || true) echo "$code $url" if (( code < 200 || code >= 400 )); then echo "Non-success status for $url" exit 1 fi doneApply the same check to the 22.04 entry (lines 19–26).
nix/tests/vm-test/default.nix (1)
441-448: Confirmed VM box URLs are reachable. All endpoints returned HTTP 200..github/workflows/update.yml (1)
17-17: Checkout v5 bump: LGTM..github/workflows/build-x86_64-darwin.yml (1)
14-14: Checkout v5 bump: LGTM..github/workflows/build-x86_64-linux.yml (1)
14-14: Checkout v5 bump: LGTM..github/workflows/build-aarch64-darwin.yml (1)
14-14: Checkout v5 bump: LGTM..github/workflows/build-aarch64-linux.yml (1)
14-14: Checkout v5 bump: LGTM.src/distribution.rs (1)
49-49: Doc tarball version update: LGTM.docs/rust-library.md (1)
43-43: Tokio link switched to latest: LGTM.src/cli/subcommand/uninstall.rs (1)
83-95: RNG API migration looks correct.Switched to rand::rng() and rng.random_range(...) appropriately; type inference for usize indices is sound here.
src/action/mod.rs (1)
165-166: Darwin pattern broadening LGTM.Matching OperatingSystem::Darwin(_) alongside MacOSX preserves macOS handling while covering payload variants.
src/action/base/create_or_insert_into_file.rs (1)
215-217: RNG update is fine.rand::rng() with rng.random::() is a drop-in replacement for the old gen::() here.
src/cli/subcommand/install/determinate.rs (1)
28-31: Darwin(_) widening is appropriate.Covers additional Darwin variants without changing behavior.
.github/workflows/release-prs.yml (2)
84-97: Confirm behavior changes in download-artifact v5.v5 altered some defaults; ensure the artifact names resolve to the same paths you mv later (directories vs files).
If needed, add explicit path:
with: name: nix-installer-x86_64-linux path: .
106-108: aws-actions/configure-aws-credentials v5 upgrade LGTM; keep OIDC permissions.You already set permissions.id-token: write; that’s required. No further changes needed.
src/action/common/place_nix_configuration.rs (1)
54-58: Broadened macOS detection LGTM.Matching MacOSX { .. } | Darwin(_) is consistent with target-lexicon updates.
src/action/macos/kickstart_launchctl_service.rs (1)
142-144: Improved error message formatting LGTM.Named placeholder makes the stderr rendering clearer; fallback for non‑UTF‑8 is correct.
Cargo.toml (1)
28-57: Dependency bumps look coherent with code changes (target-lexicon 0.13.x, rand 0.9.x, which 8.x).Given the OS pattern updates and RNG API migrations, this set appears consistent.
If CI doesn’t already, please run tests on macOS and Linux to exercise both
OperatingSystemmatch arms and therandcode paths.src/action/base/create_or_merge_nix_config.rs (2)
31-38: Error message formatting via thiserror named arg is solid.Using
{keys = .0.iter()...}with{1}for the path makes the diagnostic clearer without changing behavior.
297-299: rand 0.9 migration (rng()/random()) is correct.
rand::rng()+rng.random::<u32>()matches 0.9 APIs and theuse rand::Rng;import.src/cli/mod.rs (1)
124-128: Darwin match broadening LGTM.
OperatingSystem::Darwin(_)aligns with the target-lexicon update and other modules.src/planner/macos/mod.rs (1)
381-389: Platform check matches updated OS patterns.
MacOSX { .. } | Darwin(_)is consistent with the rest of the codebase after the bump.README.md (2)
59-60: CI docs updated to checkout@v5 — good.This mirrors workflow changes and keeps README examples aligned.
351-353: Version pin updated to v3.11.2 — consistent with docs and workflows.docs/building.md (3)
16-18: Linux build instructions updated to v3.11.2 — consistent with README.
28-30: macOS build instructions updated to v3.11.2 — consistent with README.
44-46: cargo install tag updated to v3.11.2 — consistent with other docs.src/action/base/create_group.rs (1)
30-31: Darwin(_) widening is correctBroadening to OperatingSystem::Darwin(_) is consistent with target_lexicon and matches related files.
src/action/base/create_user.rs (1)
48-49: Darwin(_) widening is correct and consistentPatterns match the intended MacOS/Darwin branches without altering behavior.
Also applies to: 134-135, 215-216
src/action/base/delete_user.rs (1)
27-28: Darwin(_) widening is correctNo behavioral changes beyond pattern shape; consistent with related modules.
Also applies to: 78-79
src/action/base/create_directory.rs (1)
304-305: Darwin(_) widening is correctThe mount and parsing branches remain logically equivalent with a broader Darwin match.
Also applies to: 334-335
src/cli/subcommand/repair.rs (1)
198-199: Darwin(_) widening is correctMatches the macOS/Sequoia-only guards without changing control flow.
Also applies to: 220-221
src/planner/mod.rs (3)
76-80: Darwin variant coverage looks goodLine 76 now accepts the payload-carrying
OperatingSystem::Darwin(_), soplatform_checkstays compatible with the updatedtarget-lexiconAPI.
188-194: Nice catch updating the default planner matchLines 188-194 broaden the Darwin arm appropriately, keeping macOS detection working after the dependency bump.
335-336: Doc link refresh appreciatedThe new Fish documentation URL on Line 335 points to the canonical “current” docs page—thanks for keeping it up to date.
src/settings.rs (3)
197-207: Defaults still align with macOS expectationsLines 197-206 update both default ID helpers to match the new Darwin signature while preserving the macOS-specific value.
227-233: Common settings Darwin pattern looks correctLines 227-233 keep
_nixbldselected for all Darwin hosts under the new enum shape.
403-406: Init settings Darwin arm stays coveredLines 403-406 mirror the new pattern so Launchd is still the default on macOS.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/action/common/create_users_and_groups.rs (1)
155-167: Both match arms execute the same logic; consider collapsingBoth arms iterate sequentially over
create_users. You can remove thematchand run the loop once to reduce duplication while keeping the concurrency block commented for future enablement.src/action/base/create_group.rs (2)
29-39: Optional: add preflight checks for macOS tools in plan()On Darwin/macOS, consider verifying
/usr/sbin/dseditgroupand/usr/bin/dsclexist to fail early, mirroring the Linux checks.Example (outside the shown lines):
if matches!(OperatingSystem::host(), OperatingSystem::MacOSX { .. } | OperatingSystem::Darwin(_)) { use std::path::Path; if !Path::new("/usr/sbin/dseditgroup").exists() { return Err(Self::error(ActionErrorKind::MissingGroupCreationCommand)); } if !Path::new("/usr/bin/dscl").exists() { return Err(Self::error(ActionErrorKind::MissingGroupDeletionCommand)); } }
95-95: Minor: redundant localuse OperatingSystem;
OperatingSystemis already imported at the module level. The inneruse OperatingSystem;lines can be removed for cleanliness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/rust-library.md(2 hunks)src/action/base/add_user_to_group.rs(4 hunks)src/action/base/create_group.rs(3 hunks)src/action/common/create_users_and_groups.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/action/base/add_user_to_group.rs
🔇 Additional comments (6)
docs/rust-library.md (2)
24-25: Docs now match the library’s env! lookupThanks for correcting the variable name and example artifact so the guidance lines up with the current code/constants.
43-43: Link update looks goodPointing to the rolling
latestTokio docs keeps the reference current without pinning to an old release.src/action/base/create_group.rs (3)
30-30: Pattern update is correct
OperatingSystem::MacOSX { .. } | OperatingSystem::Darwin(_)aligns with target-lexicon and fixes tuple-pattern mismatches in plan().
95-95: Execute path pattern match — LGTMStruct-style
MacOSXand wildcardDarwin(_)are correct here.
157-157: Revert path pattern match — LGTMConsistent and correct with the other sections.
src/action/common/create_users_and_groups.rs (1)
158-158: Approve MacOSX struct pattern and Darwin wildcard usage—LGTMNo tuple-style
OperatingSystem::MacOSX(patterns found;Darwin(_)wildcard usage is consistent across the repo.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/action/base/create_or_insert_into_file.rs (1)
213-233: Avoid truncating on rare temp-name collisions; use create_new + retryCurrent OpenOptions uses create(true) + truncate(true), which could clobber an unrelated pre-existing file if a random name collides. Prefer create_new(true) and retry on AlreadyExists to guarantee uniqueness.
Apply:
- let mut temp_file_path = parent_dir.to_owned(); - { - let mut rng = rand::rng(); - temp_file_path.push(format!("nix-installer-tmp.{}", rng.random::<u32>())); - } - let mut temp_file = OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - // If the file is created, ensure that it has harmless - // permissions regardless of whether the mode will be - // changed later (if we ever create setuid executables, - // they should only become setuid once they are owned by - // the appropriate user) - .mode(0o600) - .open(&temp_file_path) - .await - .map_err(|e| { - ActionErrorKind::Open(temp_file_path.clone(), e) - }).map_err(Self::error)?; + let (mut temp_file_path, mut temp_file); + { + let mut rng = rand::rng(); + let mut attempts = 0; + loop { + let candidate = parent_dir.join(format!("nix-installer-tmp.{}", rng.random::<u64>())); + match OpenOptions::new() + .create_new(true) + .write(true) + // Ensure harmless permissions at creation time + .mode(0o600) + .open(&candidate) + .await + { + Ok(f) => { + temp_file_path = candidate; + temp_file = f; + break; + } + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists && attempts < 8 => { + attempts += 1; + continue; + } + Err(e) => { + return Err(Self::error(ActionErrorKind::Open(candidate.clone(), e))); + } + } + } + }src/action/base/add_user_to_group.rs (1)
142-142: Prefer id -nG for robust group-membership check on Linuxgroups output varies by distro; id -nG returns a stable, space-delimited list.
Apply:
- let output = execute_command( - Command::new("groups") - .process_group(0) - .arg(&this.name) - .stdin(std::process::Stdio::null()), - ) + let output = execute_command( + Command::new("id") + .process_group(0) + .args(["-nG"]) + .arg(&this.name) + .stdin(std::process::Stdio::null()), + ) .await .map_err(Self::error)?; let output_str = String::from_utf8(output.stdout).map_err(Self::error)?; - let user_in_group = output_str.split_whitespace().any(|v| v == this.groupname); + let user_in_group = output_str.split_whitespace().any(|v| v == this.groupname);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/action/base/add_user_to_group.rs(6 hunks)src/action/base/create_directory.rs(3 hunks)src/action/base/create_file.rs(1 hunks)src/action/base/create_or_insert_into_file.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/action/base/add_user_to_group.rs (1)
src/action/mod.rs (1)
error(263-268)
🔇 Additional comments (6)
src/action/base/create_file.rs (1)
109-116: Correct error kind for missing groupSwitch to ActionErrorKind::NoGroup on group lookup miss is accurate and consistent with semantics.
src/action/base/create_or_insert_into_file.rs (1)
118-122: Correct error kind for missing groupUsing ActionErrorKind::NoGroup for group-resolution failures matches intent and other call sites.
src/action/base/create_directory.rs (2)
80-82: Correct error kind for missing groupNoGroup better reflects the failing lookup during ownership validation.
304-308: Broadened macOS detection to include Darwin variantsIncluding OperatingSystem::Darwin(_) alongside MacOSX is appropriate for target_lexicon and fixes edge OS-variant handling in mount detection.
Also applies to: 332-336
src/action/base/add_user_to_group.rs (2)
41-42: Darwin(_) pattern match expansion: goodConsistently handling OperatingSystem::Darwin(_) with MacOSX improves macOS variant coverage in plan/execute/revert.
Also applies to: 77-78, 197-198, 276-277
245-246: Cleaned up double-wrapping of errorReturning Err(Self::error(...)) directly is correct and removes redundant wrapping.
1 participant
Description
I noticed a lot of technical debt because dependabot pull requests are not getting merged.
This pull request updates everything to the latest versions and URLs.
Suggestions:
Notes:
Checklist
cargo fmtnix buildnix flake checkValidating with
install.determinate.systemsIf a maintainer has added the
upload to s3label to this PR, it will become available for installation viainstall.determinate.systems:Summary by CodeRabbit
Documentation
Chores
Tests
Bug Fixes