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.

Sign up for GitHub

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

Jump to bottom

Nix profile & installation: handle set -u installations #2194

Merged
merged 2 commits into from May 30, 2018
Merged

Nix profile & installation: handle set -u installations #2194

merged 2 commits into from May 30, 2018

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented May 30, 2018

I've verified the profile changes work properly, and the install-multi-user changes should work properly: in the block where we check the value, we export a value for it.

@grahamc
nix-daemon.sh profile script: operate under set -u
d459d33 
If the profile is sourced inside a script with `set -u`, the check for
__ETC_PROFILE_NIX_SOURCED and NIX_SSL_CERT_FILE would raise an error.
A simple guard around this check allows the script to operate under
standard environments (where it is fairly reasonable to assume USER
and HOME are set.)
@grahamc
install-multi-user: support 'set -u' runs, closes NixOS#2193
6ba1726
@grahamc
Copy link
Member Author

grahamc commented May 30, 2018

Here is how I verified the profile fix:

#!/bin/sh

set -eu

[ "${HOME:-}" != "" ] && exec -c $0 "$(dirname "$(which env)")" "$USER" "$HOME"

PATH="$1"
USER="$2"
HOME="$3"

(
    set -u
    __ETC_PROFILE_NIX_SOURCED=""
    NIX_SSL_CERT_FILE=""
    . ./scripts/nix-profile-daemon.sh.in
)

I'm building the installer again to validate it on macOS.

shlevy
shlevy approved these changes May 30, 2018
View reviewed changes
Copy link
Member

@shlevy shlevy left a comment

Choose a reason for hiding this comment

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

Looks good to me, merge when validated.

@edolstra edolstra merged commit 3be58fe into NixOS:master May 30, 2018
@grahamc grahamc deleted the nix-profile-daemon-profile-set-u branch May 30, 2018 13:47
@grahamc
Copy link
Member Author

grahamc commented May 30, 2018 •
edited

I've now built this installer on macOS and ran it, under the following scenarios:

No NIX variables:

$ env | grep NIX | wc -l
0

cert file set to an empty string:

$ env | grep NIX
NIX_SSL_CERT_FILE=

set to a bogus file:

$ env | grep NIX
NIX_SSL_CERT_FILE=bogusfile

set to a bogus file which exists but isn't certs :)

$ env | grep NIX
NIX_SSL_CERT_FILE=/Users/grahamc/test.c

and they all behaved as expected. Opening a backport PR.

@grahamc grahamc mentioned this pull request May 30, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shlevy shlevy shlevy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@grahamc @shlevy @edolstra