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

Bring nix-profile.sh in line with NixOS #452

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Conversation

wmertens
Copy link
Contributor

Use the same logic as NixOS' profile and environment setup. Closes #414 and paves the way for enabling multi-user setups from a single-user nix install.

@wmertens
Copy link
Contributor Author

@edolstra anything I can do to help deciding wether this should be merged?

@copumpkin
Copy link
Member

Is there a nice way for the common parts to be shared so this can be the last time the changes need to be carried over?

@wmertens
Copy link
Contributor Author

@copumpkin sure there is, tons of ways... But if @edolstra doesn't want to merge this, I don't see how we'll get there...

@wmertens
Copy link
Contributor Author

@edolstra is this something you'd consider merging? If so I'll fix the merge conflict?

@k2s
Copy link

k2s commented Apr 10, 2016

Could you, please, merge this ?

I had problem to run multiuser mode on ArchLinux with the /etc/profile.d/nix.sh shipped with Nix.
Everything works smooth if I use the file from this pull request.

https://aur.archlinux.org/packages/nix/
https://aur.archlinux.org/packages/nix-multiuser

@shlevy
Copy link
Member

shlevy commented Apr 10, 2016

@wmertens If you bring this up to date (ideally with common parts abstracted as per @copumpkin, but not necessary), I'll merge it.

Use the same logic as NixOS' profile and environment setup. Closes NixOS#414
@wmertens
Copy link
Contributor Author

@shlevy brought up-to-date.

A possible modularization would be a script that sets up the user profile dir and nix links, which would also be used instead of https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/programs/shell.nix#L23

@shlevy
Copy link
Member

shlevy commented Apr 11, 2016

@wmertens Just to double check, have you verified this works right with the latest nix? Also, which script in NixOS exactly is this meant to be in line with?

@wmertens
Copy link
Contributor Author

@shlevy I tested it with my unstable Darwin, and it is supposed to provide a similar env as that provided by nixos/modules/programs/shell.nix and nixos/modules/programs/environment.nix

@shlevy
Copy link
Member

shlevy commented Apr 11, 2016

Kind of weird to have the aspell stuff here IMO, but then again it's there in NixOS so I'll merge.

@shlevy shlevy merged commit af4fb6e into NixOS:master Apr 11, 2016
@wmertens
Copy link
Contributor Author

Yeah this is really about unbreaking it, and we can address declarative
environments elsewhere. I'm thinking we need a store checker that verifies
if everything is as it should be (profile and gc links, permissions,
readonly, …

On Mon, Apr 11, 2016 at 3:48 PM Shea Levy notifications@github.com wrote:

Kind of weird to have the aspell stuff here IMO, but then again it's there
in NixOS so I'll merge.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#452 (comment)

Wout.
(typed on mobile, excuse terseness)

@benley
Copy link
Member

benley commented Apr 11, 2016

Woot! (Wout!)

@wmertens wmertens deleted the patch-5 branch April 12, 2016 05:09
@edolstra
Copy link
Member

This appears to have broken the tests: http://hydra.nixos.org/build/34453788/nixlog/1/raw

@shlevy
Copy link
Member

shlevy commented Apr 13, 2016

Fixed in 7186539

@k2s
Copy link

k2s commented Apr 13, 2016

@wmertens I tested the changed script with existing ArchLinux packages without good results. Could you please comment on #879 ? I think the ArchLinux packages have to be modified.

@edolstra
Copy link
Member

Another test failure (on Darwin):

+ HOME=/private/var/tmp/nix-build-nix-1.12pre4558_12b257f.drv-0/nix-test/home
+ USER=nixbld1
+ /nix/store/m9m9ql2pbdhmbls584k1z571vr1ynfqi-bash-4.3-p42/bin/bash -e -c '. ../scripts/nix-profile.sh'
Nix: creating /private/var/tmp/nix-build-nix-1.12pre4558_12b257f.drv-0/nix-test/home/.nix-profile
mkdir: cannot create directory '/nix/var/nix/gcroots/per-user': Permission denied
FAIL: tests/nix-profile.sh

http://hydra.nixos.org/build/34557028/nixlog/1/raw

@edolstra
Copy link
Member

Note that the last time we had a multi-user-capable nix-profile.sh, it had to be reverted (#49, aac1422) because it broke single-user installs and was Linux-specific.

@k2s
Copy link

k2s commented Apr 14, 2016

the failure is related to my problems in #879.
I think that nix-profile.sh should test if /nix/var/nix/{profiles,gcroots}/per-user was made writable.
If not then it is single-user install.

it would be nice to have unified way how to setup multi-user Nix on non-NixOS systems

@benley
Copy link
Member

benley commented Apr 14, 2016

I hope we can try to keep the multi-user capability this time around, and fix these bugs as they crop up. It's worthwhile, in my opinion.

@@ -34,4 +81,7 @@ if [ -n "$HOME" ]; then
elif [ -e "$NIX_LINK/etc/ca-bundle.crt" ]; then # old cacert in Nix profile
export SSL_CERT_FILE="$NIX_LINK/etc/ca-bundle.crt"
fi

export PATH="$NIX_LINK/bin:$NIX_LINK/sbin:$__savedpath"
Copy link

Choose a reason for hiding this comment

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

You don't need quotes on this line (85).

Copy link

Choose a reason for hiding this comment

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

Same for this one. export PATH="$NIX_LINK/bin:$NIX_LINK/sbin:$__savedpath"

Copy link
Member

Choose a reason for hiding this comment

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

the quotes don't hurt anything, do they? It's never been totally clear to me exactly when you can safely omit quotes in shell variable assignments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NIX_LINK is user-provided via the HOME variable, if HOME happens to have a space in the name that would be annoying.

OTOH, the quotes don't hurt and using them always is easier and less error-prone than needing to decide when to use them.

@0xABAB
Copy link

0xABAB commented Apr 17, 2016

If we are talking about really solving problems "in the large", then I don't like the hardcoded paths in this code. Most of the paths could be generated by various methods. That is, this script should be the output of a release time build script, not something painstakingly created by a human.

I am also not sure what the reason is for all the @-symbols in your code. Perhaps you think it looks nice or something, but I would get rid of it.

@copumpkin
Copy link
Member

The @var@ entries get substituted for other values by another script

@wmertens
Copy link
Contributor Author

I think that nix-profile.sh should test if /nix/var/nix/{profiles,gcroots}/per-user was made writable. If not then it is single-user install.

Hmmm, it is rather the other way around. In a single-user install, the entire /nix tree is owned by the user…

darwin test failure

The test runs with bash -e, which is not necessary? I ignored the exit status of the mkdir on purpose, so that the error message shows every time it runs but the profile itself loads fine.

The fix is simply to put || true after the mkdir, or to adjust the test.

Note that the last time we had a multi-user-capable nix-profile.sh, it had to be reverted (#49, aac1422) because it broke single-user installs and was Linux-specific.

Single-user installs are supposed to have /nix owned by the user, no? So it should just be writeable? The Nix store assumes that gcroots is available for writing to.

Also, this version of the profile relies only on a bash-like shell and coreutils, which it ships with.

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.

nix-env doesn't auto-create per-user profile?
7 participants