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

nixos/activation: fix activation script for non-POSIX shells (retry) #46042

Merged
merged 1 commit into from Sep 5, 2018
Merged

nixos/activation: fix activation script for non-POSIX shells (retry) #46042

merged 1 commit into from Sep 5, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Sep 4, 2018 •
edited

Motivation for this change

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

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.

@Ma27
nixos/activation: fix activation script for non-POSIX shells
b3b7e51 
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
@Mic92 Mic92 added this to the 18.09 milestone Sep 4, 2018
etu
etu approved these changes Sep 4, 2018
View reviewed changes
Copy link
Contributor

@etu etu left a comment

Choose a reason for hiding this comment

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

Works fine with fish shell as my user shell 👍

@xeji xeji merged commit df05618 into NixOS:master Sep 5, 2018
xeji pushed a commit that referenced this pull request Sep 5, 2018
@Ma27 @xeji
nixos/activation: fix activation script for non-POSIX shells (#46042)
a015527 
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
Copy link
Contributor

xeji commented Sep 5, 2018

backport to 18.09: a015527

@Ma27 Ma27 deleted the fix-nixos-rebuild-for-non-posix-compliant-shells branch September 5, 2018 20:53
@xeji xeji mentioned this pull request Sep 17, 2018
Closed
etu added a commit to etu/nixpkgs that referenced this pull request Sep 18, 2018
@etu
nixos/activation: Switch from bash to sh to avoid reading users bash …
7aa0dbf 
…config

This fixes NixOS#46750. This should also work with non-POSIX shells like in NixOS#46042.
@etu etu mentioned this pull request Sep 18, 2018
Merged
9 tasks
xeji pushed a commit that referenced this pull request Sep 18, 2018
@etu @xeji
nixos/activation: Switch from bash to sh to avoid reading users bash …
4c4aff3 
…config (#46851)

This fixes #46750. This should also work with non-POSIX shells like in #46042.

(cherry picked from commit 5664e64)
@xeji
Copy link
Contributor

xeji commented Sep 18, 2018

backported to 18.09: 4c4aff3

xeji pushed a commit that referenced this pull request Sep 18, 2018
@etu @xeji
nixos/activation: Switch from bash to sh to avoid reading users bash …
5664e64 
…config (#46851)

This fixes #46750. This should also work with non-POSIX shells like in #46042.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etu etu etu approved these changes

@xeji xeji xeji approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
18.09
Development

Successfully merging this pull request may close these issues.

nixos-rebuild fails to reload user service files due to bug in switch-to-configuration.pl
5 participants
@Ma27 @xeji @etu @Mic92 @GrahamcOfBorg