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

nixos/switch-to-configuration: support shells which aren't POSIX compliant #45945

Closed
wants to merge 1 commit into from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Sep 2, 2018

fish' doesn't support the ENVVAR=foo my cmd' notation which breaks
`nixos-rebuild switch':

fish: Unsupported use of '='. To run '/nix/store/dzn7zyf1dhxlvrbpgn yn7di58zvfblrq-systemd-239/bin/systemctl'
with a modified environment, please use
'env XDG_RUNTIME_DIR=/run/user/1000 /nix/store/dzn7zyf1dhxlvrbpgnyn7di58zvfblrq-systemd-239/bin/systemctl…'

The issue can be solved by adding an `export' statement and run the
command after that.

Fixes #45897

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

…liant

`fish' doesn't support the `ENVVAR=foo my cmd' notation which breaks
`nixos-rebuild switch':

```
fish: Unsupported use of '='. To run '/nix/store/dzn7zyf1dhxlvrbpgn yn7di58zvfblrq-systemd-239/bin/systemctl'
with a modified environment, please use
'env XDG_RUNTIME_DIR=/run/user/1000 /nix/store/dzn7zyf1dhxlvrbpgnyn7di58zvfblrq-systemd-239/bin/systemctl…'
```

The issue can be solved by adding an `export' statement and run the
command after that.

Fixes NixOS#45897
@FRidh
Copy link
Member

FRidh commented Sep 2, 2018

Duplicate of #45932

@FRidh FRidh marked this as a duplicate of #45932 Sep 2, 2018
@FRidh
Copy link
Member

FRidh commented Sep 2, 2018

This would still not solve it in case of say csh.

@dezgeg
Copy link
Contributor

dezgeg commented Sep 2, 2018

Maybe su -s /bin/sh?

@Ma27
Copy link
Member Author

Ma27 commented Sep 2, 2018

closing as duplicate, let's fix this properly in #45932 ...

@Ma27 Ma27 closed this Sep 2, 2018
@Ma27 Ma27 deleted the nixos-rebuild-fish-support branch September 2, 2018 16:23
Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Sep 4, 2018
This fixes an issue with shells like fish that are not fully POSIX
compliant. The syntax `ENV=val cmd' doesn't work properly in there.

This issue has been addressed in NixOS#45932 and NixOS#45945, however it has been
recommended to use a single shell (`stdenv.shell' which is either
`bash' or `sh') to significantly reduce the maintenance overload in the
future.

See NixOS#45897 (comment)

Fixes NixOS#45897

/cc @FRidh @xaverdh @etu
xeji pushed a commit that referenced this pull request Sep 5, 2018
This fixes an issue with shells like fish that are not fully POSIX
compliant. The syntax `ENV=val cmd' doesn't work properly in there.

This issue has been addressed in #45932 and #45945, however it has been
recommended to use a single shell (`stdenv.shell' which is either
`bash' or `sh') to significantly reduce the maintenance overload in the
future.

See #45897 (comment)

Fixes #45897

/cc @FRidh @xaverdh @etu

(cherry picked from commit df05618)
xeji pushed a commit that referenced this pull request Sep 5, 2018
This fixes an issue with shells like fish that are not fully POSIX
compliant. The syntax `ENV=val cmd' doesn't work properly in there.

This issue has been addressed in #45932 and #45945, however it has been
recommended to use a single shell (`stdenv.shell' which is either
`bash' or `sh') to significantly reduce the maintenance overload in the
future.

See #45897 (comment)

Fixes #45897

/cc @FRidh @xaverdh @etu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants