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

Lua: moving some package to the luarocks whitelist #55305

Merged
merged 5 commits into from Feb 12, 2019

Conversation

teto
Copy link
Member

@teto teto commented Feb 6, 2019

Motivation for this change

Should be easier to maintain in the long run.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@teto
Copy link
Member Author

teto commented Feb 6, 2019

CC @7c6f434c

@teto
Copy link
Member Author

teto commented Feb 6, 2019

In my tests 6 months ago with similar packages it worked out fine. I am just worried for mac users but luarocks should adapt its buildprocess accordingly.

@7c6f434c
Copy link
Member

7c6f434c commented Feb 6, 2019

@GrahamcOfBorg build luaPackages.lgi

@teto
Copy link
Member Author

teto commented Feb 7, 2019

regarding the darwin failure, the chmod is done by luarocks so I wonder what's wrong, maybe it gets a wrong shell sh: sw_vers: command not found.
Is the build flaky https://hydra.nixos.org/job/nixpkgs/gcc8/luaPackages.lgi.x86_64-darwin ?

@7c6f434c
Copy link
Member

These are stop signs, not crosses — there are two successful builds and two manually cancelled ones

@7c6f434c
Copy link
Member

@GrahamcOfBorg build luaPackages.basexx

@7c6f434c
Copy link
Member

@GrahamcOfBorg build luaPackages.basexx luaPackages.mpack neovim

@7c6f434c
Copy link
Member

chmod problem is still there…

@teto
Copy link
Member Author

teto commented Feb 11, 2019

@GrahamcOfBorg build luaPackages.basexx luaPackages.mpack

@7c6f434c
Copy link
Member

Apparently the patching of sw_vers doesn't happen

@7c6f434c
Copy link
Member

Apparently we let some wrong chmod into the build; the chmod command comes from stdenv's unpackPhase (can be turned off via dontMakeSourcesWritable)

@alyssais
Copy link
Member

Coming from here:

https://github.com/NixOS/nixpkgs/blob/ae8ae1de154b433dcefa3d23aa3905579179b474/pkgs/development/tools/misc/luarocks/setup-hook.sh#L12

${unzip} is expanding to nothing, and so /bin is added to PATH.

@7c6f434c
Copy link
Member

@alyssais Thanks for the explanation.

@7c6f434c
Copy link
Member

@GrahamcOfBorg build luaPackages.basexx luaPackages.mpack luaPackages.lgi neovim

@teto
Copy link
Member Author

teto commented Feb 12, 2019

@GrahamcOfBorg build luaPackages.basexx luaPackages.mpack luaPackages.lgi neovim

@7c6f434c
Copy link
Member

@GrahamcOfBorg build luaPackages.basexx luaPackages.mpack luaPackages.lgi neovim

Aarch64 failure seems to be ENOSPC while building LLVM for Mesa for lgi

@teto
Copy link
Member Author

teto commented Feb 12, 2019

I asked on nixos-dev for the machine to be fixed so hopefully there should be no space problem anymore. if darwin passes, I may move unzip to nativeBuildInputs. Is it going to be propagated ? in case someone wants to run luarocks unpack <src.rock> (is nativePropagatedBuildInputs a thing ?)

@7c6f434c
Copy link
Member

7c6f434c commented Feb 12, 2019

The manual says you spell it wrong. propagatedNativeBuildInputs as per https://nixos.org/nixpkgs/manual/#ssec-stdenv-dependencies

@7c6f434c 7c6f434c merged commit 2a2cf5b into NixOS:master Feb 12, 2019
@teto teto deleted the lua_whitelist branch February 13, 2019 03:21
vcunat added a commit that referenced this pull request Mar 4, 2019
This reverts commit c01fe37.
See the reverted commit on GitHub for discussion.  /cc PR #55305.
vcunat added a commit that referenced this pull request Mar 4, 2019
This reverts commit c01fe37.
See the reverted commit on GitHub for discussion.  /cc PR #55305.

(cherry picked from commit 3e442fd)
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

5 participants