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

Allow customizing the system config to include in the tarball #260

Merged
merged 2 commits into from
Jul 3, 2023

Conversation

terlar
Copy link
Contributor

@terlar terlar commented Jun 19, 2023

If you build your own custom tarballs this is useful.

@terlar
Copy link
Contributor Author

terlar commented Jun 20, 2023

@nzbr I hope the ping is not disturbing, just wanted to make sure this one was not missed, seems it requires approval to run the workflows as well. But I have tested this locally and it works.

I don't think it is a controversial change, however, I was slightly unsure about the option name.

modules/build-tarball.nix Outdated Show resolved Hide resolved
modules/build-tarball.nix Outdated Show resolved Hide resolved
modules/build-tarball.nix Outdated Show resolved Hide resolved
modules/build-tarball.nix Outdated Show resolved Hide resolved
@nzbr nzbr added the enhancement New feature or request label Jun 20, 2023
@terlar terlar force-pushed the custom-config-path branch 2 times, most recently from a772b21 to a5bd486 Compare June 21, 2023 00:16
@terlar
Copy link
Contributor Author

terlar commented Jun 21, 2023

Not sure if this is cleaner or not, I had to add wrapping parenthesis around the whole string to get it to work

@terlar
Copy link
Contributor Author

terlar commented Jun 22, 2023

Would it make sense to chown these files to the defaultUser and add write permissions? I just tested this and in order to modify these after the install I first had to do that, or edit the files via sudo and force write, since it didn't have write permissions configured.

sudo chown -R ${config.wsl.defaultUser} etc/nixos
sudo chmod -R u+w etc/nixos

What do you think?

This should go for both cases. At least the write permission seems like something you'd like even if the files are owned by root.

@SuperSandro2000
Copy link
Member

This is the standard nixos configuration and I am not sure if we should change that.

@nzbr
Copy link
Member

nzbr commented Jun 26, 2023

I don't think the default user should be able to write the config, but giving root write permissions on the files would definitely be a good idea (and something I wanted to make an issue for multiple times, but always forgot)

@terlar
Copy link
Contributor Author

terlar commented Jun 26, 2023

I pushed another commit, giving the root write permissions to the /etc/nixos files. I opted for string interpolation again for this section to avoid parenthesis overload. Let me know if you prefer another style.

@SuperSandro2000 SuperSandro2000 merged commit 0982e9a into nix-community:main Jul 3, 2023
17 checks passed
@terlar terlar deleted the custom-config-path branch July 3, 2023 19:16
terlar added a commit to terlar/NixOS-WSL that referenced this pull request Oct 2, 2023
This was previously implemented in nix-community#260, but lost in the migration off
the installer tarball.
terlar added a commit to terlar/NixOS-WSL that referenced this pull request Oct 3, 2023
This was previously implemented in nix-community#260, but lost in the migration off
the installer tarball.
terlar added a commit to terlar/NixOS-WSL that referenced this pull request Nov 13, 2023
This was previously implemented in nix-community#260, but lost in the migration off
the installer tarball.
terlar added a commit to terlar/NixOS-WSL that referenced this pull request Nov 13, 2023
This was previously implemented in nix-community#260, but lost in the migration off
the installer tarball.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants