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/users-groups: Use user name not attribute name for /etc/profiles/… #89951

Merged
merged 1 commit into from Apr 5, 2021

Conversation

KoviRobi
Copy link
Contributor

@KoviRobi KoviRobi commented Jun 9, 2020

This cropped up, because I have a set-up where my work username is
different to my home desktop username, and I am using a parameterized
config for both, so I have something akin to

config.users.users.default-user = ...;

and using

config.users.users.default-user.{name, home}

in certain places to cope with this. Noticed my home-manager bought in
packages (which use the users.users..packages hence NixOS issue
not home-manager) weren't present.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Rebuild my NixOS system with the change, things work and I can find my packages
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@grahamc
Copy link
Member

grahamc commented Jun 9, 2020

I don't have a strong opinion on this, but have the question: is the canonical-according-to-Nix name the attribute name, or is it the string in the option? I would say attribute name, which makes the current behavior correct.

@KoviRobi
Copy link
Contributor Author

KoviRobi commented Jun 9, 2020

Perhaps attr-name or just _ would be better, not sure.
Edit: Instead of nix-name

@KoviRobi
Copy link
Contributor Author

KoviRobi commented Jun 10, 2020

I believe it's the string name, which by default is the same as the attribute name [1] so it doesn't matter in most circumstances, except I have a weird setup where on my work machine my username has a different name to my home setup, so here the attribute name is default-user, and I use config.users.users.default-user.name and config.users.users.default-user.home in bits of the config

[1]

[ { name = mkDefault name;

@KoviRobi KoviRobi force-pushed the nixos-user-name-not-attr-name branch 2 times, most recently from 5da8e45 to c0ae824 Compare June 10, 2020 11:58
@KoviRobi KoviRobi marked this pull request as draft June 11, 2020 19:34
@KoviRobi
Copy link
Contributor Author

KoviRobi commented Jul 6, 2020

I marked this as draft because I noticed

Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in numeric comparison (<=>) at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 253.
Use of uninitialized value in join or string at /nix/store/w5s7s1nnzsmzdqh06abysffrj2m0nrwf-update-users-groups.pl line 252.

in my PC's update, which is on 0f5ce2f, I'll see if updating it changes anything.

@KoviRobi
Copy link
Contributor Author

KoviRobi commented Jul 7, 2020

Hmm, fixed that problem, turns out it was my /etc/passwd containing

\0\0...\0::::::

that is, a lot of null characters, followed by an empty entry. Must have been some corruption at some point, that I caused, which would explain why I didn't see it on any other machine.

@KoviRobi KoviRobi force-pushed the nixos-user-name-not-attr-name branch from c0ae824 to c0fed20 Compare July 10, 2020 16:57
@KoviRobi KoviRobi marked this pull request as ready for review July 10, 2020 16:57
@KoviRobi
Copy link
Contributor Author

As above, the users.users.<name>.name is set on https://github.com/NixOS/nixpkgs/blob/c0fed202c43ae1091f0d0d8ec9678bdbabde02e6/nixos/modules/config/users-groups.nix#L282-L286

and for users.groups.<name>.name similarly
https://github.com/NixOS/nixpkgs/blob/c0fed202c43ae1091f0d0d8ec9678bdbabde02e6/nixos/modules/config/users-groups.nix#L338-L340

The users.users.<name>.name is already used in these places:

@KoviRobi
Copy link
Contributor Author

Ah, I see #110627 implemented some of the changes from here (and closed #107353), perhaps it's worth looking over the remaining changes? I have updated this branch

@4z3

…s/...

This cropped up, because I have a set-up where my work username is
different to my home desktop username, and I am using a parameterized
config for both, so I have something akin to

    config.users.users.default-user = ...;

and using

    config.users.users.default-user.{name, home}

in certain places to cope with this. Noticed my home-manager bought in
packages (which use the users.users.<name>.packages hence NixOS issue
not home-manager) weren't present.
@Lassulus Lassulus merged commit e2080b3 into NixOS:master Apr 5, 2021
@KoviRobi KoviRobi deleted the nixos-user-name-not-attr-name branch October 10, 2022 09:50
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

5 participants