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

only add existing directories to PATH #27216

Closed
wants to merge 1 commit into from
Closed

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jul 7, 2017

Motivation for this change

Fix #27195

Things done
  • Tested in a NixOS VM (with bash and fish)
  • Tested via one or more NixOS test(s) if existing and applicable for the change
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

Unverified

This user has not yet uploaded their public signing key.
@edolstra
Copy link
Member

edolstra commented Jul 7, 2017

We don't check for existence on purpose so that you don't need to log out when a path element is first created. For example, if ~/.nix-profile/bin doesn't exist, it should still be added to $PATH so that you don't need to log in again after running nix-env -i for the first time.

@0xABAB
Copy link
Contributor

0xABAB commented Jul 7, 2017

The consequence of this is that users need to relogin after a configuration change has been done by the administrator. In short, this is not just an improvement.

I would like to see designs implemented which do not break the existing semantics. One such design is patching fish (there are not that many programs that work like fish regarding the PATH). There are other designs.

@0xABAB
Copy link
Contributor

0xABAB commented Jul 7, 2017

@edolstra haha, we just wrote the exact same thing :)

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jul 7, 2017

I understand, that is convenient. Well, I can't imagine how to patch fish without removing the warning entirely. Maybe we could patch foreign-env instead.

@rnhmjoj rnhmjoj closed this Jul 7, 2017
@rnhmjoj rnhmjoj deleted the path branch September 12, 2017 23:03
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.

None yet

3 participants