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

Better support users/groups existing before install #238

Merged

Conversation

Hoverbear
Copy link
Contributor

@Hoverbear Hoverbear commented Feb 6, 2023

Description

This PR makes it so that if users/groups exist already on a machine when we attempt an install, and they're the UIDs/GIDs we expect, we mark them as already done. (This also means we don't remove them on uninstall)

Part of #35

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

@Hoverbear Hoverbear self-assigned this Feb 6, 2023
@Hoverbear Hoverbear added this to the v0.2.1 milestone Feb 7, 2023
@Hoverbear Hoverbear requested a review from cole-h February 8, 2023 19:59
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

I think this looks fine.

@@ -37,7 +35,7 @@ impl CreateUsersAndGroups {
settings.nix_build_group_id,
)
})
.collect();
.collect::<Result<_, _>>()?;
Copy link
Member

Choose a reason for hiding this comment

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

That's a weird type signature... I don't often see collecting into Result<_, _> 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fun to can turn a Vec<Result<_,_>> into a Result<Vec<_>,_>

@Hoverbear
Copy link
Contributor Author

This partially improves #218 but doesn't quite cover it.

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