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

lua wrapper: Fix bash error #73946

Merged
merged 1 commit into from Nov 23, 2019
Merged

lua wrapper: Fix bash error #73946

merged 1 commit into from Nov 23, 2019

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Nov 23, 2019 •
edited

Motivation for this change

Recently, we made it harder for external code to use some stdenv-only bash
variables by unsetting them in

nixpkgs/pkgs/stdenv/generic/setup.sh

Lines 574 to 578 in 9d3911f

# Unset setup-specific declared variables
unset allPlatOffsets
unset pkgBuildAccumVars pkgHostAccumVars pkgTargetAccumVars pkgAccumVarVars
unset pkgBuildHookVars pkgHostHookVars pkgTargetHookVars pkgHookVarVars
unset propagatedDepFilesVars
But Lua's withPackages was sourcing some
setup hooks in

nixpkgs/pkgs/development/interpreters/lua-5/wrapper.nix

Lines 23 to 27 in 9d3911f

. "${makeWrapper}/nix-support/setup-hook"
# get access to lua functions
. ${lua}/nix-support/setup-hook
, which required those bash variables.

I say great! We caught something bad: Lua should use normal dependencies, even
though that is harder with buildEnv. Now it works that way, and everything is
fine.

CC @matthewbauer

Fixes #73887

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@Ericson2314
lua wrapper: Fix bash error
a610a38 
Recently, we made it harder for external code to use some stdenv-only bash
variables by unsetting them in [1] But Lua's `withPackages` was sourcing some
setup hooks in [2], which required those bash variables.

I say great! We caught something bad: Lua should use normal dependencies, even
though that is harder with `buildEnv`. Now it works that way, and everything is
fine.

[1]: https://github.com/NixOS/nixpkgs/blob/9d3911f806034197bb7ace586cc3696ffce7f447/pkgs/stdenv/generic/setup.sh#L574-L578

[2]: https://github.com/NixOS/nixpkgs/blob/9d3911f806034197bb7ace586cc3696ffce7f447/pkgs/development/interpreters/lua-5/wrapper.nix#L23-L27

CC @matthewbauer
@Ericson2314 Ericson2314 merged commit 505dcb3 into NixOS:master Nov 23, 2019
@Ericson2314 Ericson2314 mentioned this pull request Nov 23, 2019
Merged
10 tasks
@Ericson2314 Ericson2314 deleted the fix-lua-env branch November 23, 2019 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@teto teto Awaiting requested review from teto

Assignees
No one assigned
Labels
6.topic: lua 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

neovim broken by recent (stdenv?) refactoring in master
2 participants
@Ericson2314 @matthewbauer