Skip to content

Add option to enable the service for all users#83

Open
Ten0 wants to merge 6 commits intonix-community:masterfrom
Ten0:enable_for_all_users
Open

Add option to enable the service for all users#83
Ten0 wants to merge 6 commits intonix-community:masterfrom
Ten0:enable_for_all_users

Conversation

@Ten0
Copy link
Copy Markdown
Collaborator

@Ten0 Ten0 commented Jul 3, 2024

Resolves #69
Resolves #65

What it does:

  • When the services.vscode-server.enableForUsers.enable = true is specified and using nixos-vscode-server as a NixOS module, uses tmpfiles to setup the appropriate symlinks to enable the service for all regular users
  • Adds relevant instructions to the README
  • Also adds instructions to the README to add instructions about how to use nixos-vscode-server as a home-manager module in combination with flakes

@fsnkty
Copy link
Copy Markdown

fsnkty commented Oct 16, 2024

Just as a sanity check is there a reason the scope of this is only to enable the service for all users?
is enabling the ability to specify the specific users to enable the service on a big change?
thanks for making this PR

@Ten0
Copy link
Copy Markdown
Collaborator Author

Ten0 commented Oct 16, 2024

No that wouldn't be a big change and it would also make sense.
I have pretty much lost hope that this is ever getting merged though so I'm not investing more time in this, as there is no active maintainer on this project (#67 (comment), but then I got ghosted, which honestly is understandable as the xz story happened at about the same time so they probably got suddenly unadventurous 😅).

I would be willing to merge a PR to this branch to my fork though 😊

@fsnkty
Copy link
Copy Markdown

fsnkty commented Oct 16, 2024

No that wouldn't be a big change and it would also make sense. I have pretty much lost hope that this is ever getting merged though so I'm not investing more time in this, as there is no active maintainer on this project (#67 (comment), but then I got ghosted, which honestly is understandable as the xz story happened at about the same time so they probably got suddenly unadventurous 😅).

understandable, maybe its time to look into finding the right person to ask about managing nix-community here given they seem both unable to work on the repo and unwilling (?) to allow someone else to.

unfortunate, guess I'll get to solving this elsewhere

@msteen
Copy link
Copy Markdown
Collaborator

msteen commented Oct 17, 2024

@Ten0 Due to various reasons I don't want to continue working on this project. I asked before (in matrix), without success, but now someone looked into it and turned out I wasn't admin of my own project (explained why I could not find the settings page no matter what I tried). I just invited you as maintainer.

@Ten0
Copy link
Copy Markdown
Collaborator Author

Ten0 commented Oct 17, 2024

Thanks! 😊
I'll do my best 😊

@moritztim
Copy link
Copy Markdown

moritztim commented Oct 18, 2024

Hey, thanks so much for taking over! so is this getting merged?

Edit: I just saw that this was a very recent development, so take your time haha

@Ten0 Ten0 force-pushed the enable_for_all_users branch 5 times, most recently from 8d73bcc to be0d05d Compare October 19, 2024 18:10
@Ten0 Ten0 force-pushed the enable_for_all_users branch from be0d05d to b5a62ed Compare October 19, 2024 18:10
@Ten0
Copy link
Copy Markdown
Collaborator Author

Ten0 commented Oct 19, 2024

@fsnkty does the new proposed interface look good to you?

Copy link
Copy Markdown

@fsnkty fsnkty left a comment

Choose a reason for hiding this comment

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

documentation comments are suggestions,
expecting the user attribute name to match its .name value however really should be addressed, I don't believe its an uncommon practice

otherwise all seems good to me bar some practices I don't fully understand that aren't related to this PR.

thank you for working on this

Comment thread modules/vscode-server/default.nix Outdated
Comment thread README.md
Comment thread modules/vscode-server/module.nix
@Ten0 Ten0 force-pushed the enable_for_all_users branch 2 times, most recently from c0b7edb to d7ad842 Compare October 20, 2024 10:47
@Ten0 Ten0 force-pushed the enable_for_all_users branch from d7ad842 to 725f14e Compare October 20, 2024 10:50
@Ten0 Ten0 requested a review from fsnkty October 20, 2024 11:05
let
forEachUser = ({ path, file }: builtins.listToAttrs
(builtins.map
(username: let user = config.users.users.${username}; in {
Copy link
Copy Markdown

@fsnkty fsnkty Oct 20, 2024

Choose a reason for hiding this comment

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

I'm still unable to get this to work as expected both config.users.users.foo.name and "bar" fail.
when e.g.. config.users.users.foo.name = "bar"
for reference users.groups.<name>.members handles this correctly, may be useful to take a look at that.
thank you

Copy link
Copy Markdown
Collaborator Author

@Ten0 Ten0 Oct 21, 2024

Choose a reason for hiding this comment

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

Can you please expand on how it "doesn't work as expected"?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you please expand on how it "doesn't work as expected"?

I have config.users.users.main.name = "fsnkty"
setting services.vscode-server.enableForUsers.users = [ config.users.users.main.name ];
breaks this in that

       error: attribute 'fsnkty' missing
       at /nix/store/fbgf31l3gy506di92bbw12aw95jg4cs6-source/modules/vscode-server/default.nix:20:37:
           19|             (builtins.map
           20|               (username: let user = config.users.users.${username}; in {
             |                                     ^
           21|                 name = "${user.home}/${path}";

giving "fsnkty" breaks in the same way, the string "main" can be used but that's non ideal and doesn't behave similarly to any other option that Ive encountered which expects a list of users.

this workaround in using the user attribute name as a string instead of its given name is non ideal because now changing that value is separate from changing it anywhere else a user is specified or named

@RyzeNGrind
Copy link
Copy Markdown

Anything blocking this from getting merged?

@RyzeNGrind RyzeNGrind mentioned this pull request May 3, 2025
@tfc
Copy link
Copy Markdown

tfc commented Oct 8, 2025

Can this be merged? @fsnkty @Ten0

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.

Declaratively enable user service in NixOS module Instructions unclear with home-manager managed as a flake

6 participants