Handle use-xdg-base-directories for profile link by balsoft · Pull Request #7929 · NixOS/nix

balsoft · 2023-03-01T10:46:50Z

Motivation

#5588 broke the PATH setting logic in profile.sh: #5588 (comment)

This PR attempts to fix that by checking the use-xdg-base-directories config option and deciding whether to use the XDG-compliant location or the old one based on that.

Context

#5588 introduced a use-xdg-base-directories option which, when enabled, makes Nix use the new locations instead of ~/.nix-profile, ~/.nix-channel and ~/.nix-defexpr.
However, the accompanying profile.sh change was not quite correct: it chose the new location if the old one didn't exist.
Checking the use-xdg-base-directories should enable us to handle this properly (only adding the relevant profile link to $PATH).

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests - tests/nixos/*
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or incompatible change: updated release notes

edolstra · 2023-03-01T12:19:44Z

scripts/nix-profile-daemon.sh.in

-            printf "$warning Profiles do not match. You should manually migrate from %s to %s.\n" "$NIX_LINK" "$NIX_LINK_NEW" 1>&2
-        fi
+NIX_LINK="$HOME/.nix-profile"
+if [[ $(PATH="$PATH:@localstatedir@/nix/profiles/default/bin" nix show-config use-xdg-base-directories) == true ]]; then


Ideally we shouldn't call nix (or any non-shell builtin) inside profile scripts since it slows down every shell invocation...

Hm, true. Each invocation of nix show-config use-xdg-base-directories takes about 17 ms, which is quite a lot :(

Would it be an acceptable compromise to just add both locations to $PATH then?

I guess 17 ms is fine for login shells. That's probably preferrable over polluting $PATH with non-existent paths.

Yeah I think it is fine for login shells.

scripts/nix-profile-daemon.sh.in

cole-h

I'm slightly meh on the idea of needing to invoke nix on shell startup (even if it's "only" for login shells), but if this is decided as the accepted solution, we need to make sure to pass --extra-experimental-features (or use a legacy CLI invocation, if one exists...?).

scripts/nix-profile-daemon.sh.in

scripts/nix-profile.sh.in

fricklerhandwerk · 2023-04-05T08:53:56Z

Triaged in the Nix team meeting 2023-03-31:

@Ericson2314: we need a plan to make XDG the default
- it's a lot of work maintaining both
to discuss

nixos-discourse · 2023-04-05T08:55:08Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-03-31-nix-team-meeting-minutes-45/27002/1

fricklerhandwerk · 2023-04-05T10:55:55Z

Discussed in the Nix team meeting 2023-04-03:

The team agrees we don't want to invoke Nix to check the config option, but no other decision was recorded in the meeting notes
@Ericson2314 thinks the full solution will be getting rid of this config option altogether
- then we are clearly going towards one version (e.g. check per XDG dir if they exist, or neither exists)
- concretely, this would mean that just checking the state of the filesystem is actually correct, rather than mostly correct.

nixos-discourse · 2023-04-05T11:06:59Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-04-03-nix-team-meeting-minutes-46/27008/1

thufschmitt · 2023-07-31T08:35:40Z

Re-discussed during the Nix team meeting. Since this isn't urgent any more (a less-correct but acceptable fix having being merged) @Ericson2314 will try an alternative approach that doesn't involve calling nix (with --extra-experimental-features 😱 ) in the init script. Closing that one in the meantime.

Discussion log

Not that urgent since another fix for the original issue landed
    That one is more principled, though
    But involves calling Nix in the shell startup script, which is meh
    @regnat: Also uses xp features in the init script, which is a no-go
    @ericson2314: We could make the usage of XDG dirs dependant on whether the corresponding files exist or not, which would make the current (master) solution totally correct, and would sidestep the problem altogether
        @ericson2314 will try doing that
        Will also make sure the default value indicates this.
    Will close that PR in the meantime

nixos-discourse · 2023-07-31T08:43:50Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-24-nix-team-meeting-minutes-74/31116/1

balsoft requested a review from edolstra as a code owner March 1, 2023 10:46

balsoft force-pushed the fix-path-in-shell-profile branch from 2fef0f4 to a2e6820 Compare March 1, 2023 10:47

balsoft mentioned this pull request Mar 1, 2023

Check if NIX_LINK_NEW exists instead of checking that NIX_LINK doesn't exist #7925

Merged

7 tasks

edolstra reviewed Mar 1, 2023

View reviewed changes

scripts/nix-profile-daemon.sh.in Outdated Show resolved Hide resolved

balsoft force-pushed the fix-path-in-shell-profile branch from a2e6820 to 9bedfd6 Compare March 1, 2023 12:23

abathur mentioned this pull request Mar 1, 2023

Issue with Installing Nix After MacOS Update (Nix stopped working) #7880

Open

3 tasks

balsoft mentioned this pull request Mar 2, 2023

nix-shell -p nix-info --run "nix-info -m" fails after installation. #7937

Open

3 tasks

cole-h suggested changes Mar 2, 2023

View reviewed changes

scripts/nix-profile-daemon.sh.in Outdated Show resolved Hide resolved

scripts/nix-profile.sh.in Outdated Show resolved Hide resolved

fricklerhandwerk added the regression Something doesn't work anymore label Mar 3, 2023

Handle use-xdg-base-directories for profile link

56df2a4

balsoft force-pushed the fix-path-in-shell-profile branch from 9bedfd6 to 56df2a4 Compare March 13, 2023 14:39

Ericson2314 removed the regression Something doesn't work anymore label Mar 31, 2023

roberth added the profiles Versioned gc root symlinks; nix profile, nix-env label Mar 31, 2023

fricklerhandwerk added the regression Something doesn't work anymore label Apr 5, 2023

balsoft mentioned this pull request May 23, 2023

nix: 2.13.3 -> 2.15.1 NixOS/nixpkgs#233439

Merged

12 tasks

thufschmitt closed this Jul 31, 2023

Uh oh!

Comments

Conversation

balsoft commented Mar 1, 2023

Motivation

Context

Checklist for maintainers

Uh oh!

edolstra Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

balsoft Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

balsoft Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edolstra Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Mar 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cole-h left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fricklerhandwerk commented Apr 5, 2023

Uh oh!

nixos-discourse commented Apr 5, 2023

Uh oh!

fricklerhandwerk commented Apr 5, 2023

Uh oh!

nixos-discourse commented Apr 5, 2023

Uh oh!

thufschmitt commented Jul 31, 2023

Uh oh!

nixos-discourse commented Jul 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

balsoft Mar 1, 2023 •

edited

Loading