Skip to content

Conversation

@lblasc
Copy link
Contributor

@lblasc lblasc commented Jun 6, 2020

Motivation for this change

Update luarocks generated packages and add lyaml - lua yaml parser. I've also found that lua pulseaduio package was missing from generated-packages.nix.

I've tested most of the updated packages.

cc @teto @vcunat

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 nixpkgs-review --run "nixpkgs-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.

@lblasc lblasc changed the title lyaml: init at 6.2.5-1, update all generated packages lyaml: init at 6.2.5-1, update all luarocks generated packages Jun 6, 2020
@ofborg ofborg bot added 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Jun 6, 2020
@teto
Copy link
Member

teto commented Jun 6, 2020

could you not update the lua packages version ? (but just change the url so that it uses mirrors instead). I am afraid this would break other things.
As for the pulseaudio one, I wanted to move it to generated too see #78565 but didn't have the time: you would need to remove it from lua-packages.nix too cc @doronbehar

@lblasc lblasc force-pushed the bump-lua-packages branch from 7b7b518 to e34e9ce Compare June 6, 2020 13:40
@lblasc
Copy link
Contributor Author

lblasc commented Jun 6, 2020

@teto I've checked this pull request with: nix-shell -p nixpkgs-review --run "nixpkgs-review pr 89632" all dependencies should build fine (at least on x86_64).
But I didn't updated luv, I saw @doronbehar is working on this in #80528
Additionally I've removed pulseaudio from the lua-packages.nix as you requested.

In my opinion this is safe to merge.

@lblasc lblasc changed the title lyaml: init at 6.2.5-1, update all luarocks generated packages lyaml: init at 6.2.5-1, update almost all luarocks generated packages Jun 6, 2020
@teto
Copy link
Member

teto commented Jun 6, 2020

@GrahamcOfBorg build neovim luaPackages.lyaml luaPackages.cqueues

@teto
Copy link
Member

teto commented Jun 6, 2020

builds look ok but at runtime there may be issues. I will merge anyway.

100 packages built:
awesome gnvim knot-resolver lua51Packages.argparse lua51Packages.busted lua51Packages.cassowary lua51Packages.cqueues lua51Packages.http lua51Packages.ldoc lua51Packages.lpeglabel lua51Packages.lua-lsp lua51Packages.lua-messagepack lua51Packages.luacheck lua51Packages.luacov lua51Packages.luaposix lua51Packages.luasec lua51Packages.luasql-sqlite3 lua51Packages.luassert lua51Packages.luautf8 lua51Packages.lyaml lua51Packages.mpack lua51Packages.nvim-client lua51Packages.penlight lua51Packages.pulseaudio lua51Packages.rapidjson lua51Packages.std_normalize luaPackages.argparse luaPackages.busted luaPackages.cassowary luaPackages.cqueues luaPackages.http luaPackages.ldoc luaPackages.lpeglabel luaPackages.lua-lsp luaPackages.lua-messagepack luaPackages.luacheck luaPackages.luacov luaPackages.luaposix luaPackages.luasec luaPackages.luasql-sqlite3 luaPackages.luassert luaPackages.luautf8 luaPackages.lyaml luaPackages.mpack luaPackages.nvim-client luaPackages.penlight luaPackages.pulseaudio luaPackages.rapidjson luaPackages.std_normalize lua53Packages.argparse lua53Packages.busted lua53Packages.cassowary lua53Packages.cqueues lua53Packages.http lua53Packages.ldoc lua53Packages.lpeglabel lua53Packages.lua-lsp lua53Packages.lua-messagepack lua53Packages.luacheck lua53Packages.luacov lua53Packages.luaposix lua53Packages.luasec lua53Packages.luasql-sqlite3 lua53Packages.luassert lua53Packages.luautf8 lua53Packages.lyaml lua53Packages.mpack lua53Packages.nvim-client lua53Packages.penlight lua53Packages.pulseaudio lua53Packages.rapidjson lua53Packages.std_normalize luajitPackages.argparse luajitPackages.busted luajitPackages.cassowary luajitPackages.cqueues luajitPackages.http luajitPackages.ldoc luajitPackages.lpeglabel luajitPackages.lua-lsp luajitPackages.lua-messagepack luajitPackages.luacheck luajitPackages.luacov luajitPackages.luaposix luajitPackages.luasec luajitPackages.luasql-sqlite3 luajitPackages.luassert luajitPackages.luautf8 luajitPackages.lyaml luajitPackages.mpack luajitPackages.nvim-client luajitPackages.penlight luajitPackages.pulseaudio luajitPackages.rapidjson luajitPackages.std_normalize mudlet neovim-qt neovim-unwrapped prosody sile

I will try to update the lua documentation #55302 this WE.

@teto teto merged commit bd400bd into NixOS:master Jun 6, 2020
@doronbehar
Copy link
Contributor

As for the pulseaudio one, I wanted to move it to generated too see #78565 but didn't have the time: you would need to remove it from lua-packages.nix too cc @doronbehar

I don't care if my pulseaudio will move from one place to another, as long as I'm pinged to test something that's supposed to work. @teto I think the issue I wrote about here is orthogonal to #89145 (comment) - luaPackages.pulseaudio can't be compiled with overriding makeFlags I think and buildLuarocksPackage doesn't support these.

@lblasc lblasc deleted the bump-lua-packages branch June 7, 2020 06:46
vcunat added a commit to vcunat/nixpkgs that referenced this pull request Jun 7, 2020
This reverts commit 47ad7d3.
The fix isn't needed after the update contained in PR NixOS#89632.
@vcunat
Copy link
Member

vcunat commented Jun 7, 2020

The cqueues update should be safe enough, I believe.

@teto
Copy link
Member

teto commented Jun 7, 2020

@doronbehar sry I forgot about that. Luarocks make buildSystem can accept variables passed by extraVariables or via luarocks make (while the cmake build system is closed). So this could be a simple override.

@doronbehar
Copy link
Contributor

My lua.pkgs.pulseaudio package broke today after a system update. I've just made it through via an ugly overlay... I'm working on a nicer fix.

@doronbehar
Copy link
Contributor

Sigh.... I really don't know what to do, it's working when I build it with a makefile, and when I use buildLuaPackage but not with buildLuarocksPackage.

@doronbehar
Copy link
Contributor

@teto could it be something with the Makefile? https://github.com/doronbehar/lua-pulseaudio/blob/v0.2/Makefile

doronbehar added a commit to doronbehar/nixpkgs that referenced this pull request Jun 8, 2020
From some reason, the package is not usable if it's built with
`buildLuarocksPackage`. Adding `extraVariables` as in `makeFlags`
doesn't work. See:
NixOS#89632 (comment)

Also, remove it from luarocks-packages.csv so hopefully no one will
touch it in the future.
@teto
Copy link
Member

teto commented Jun 8, 2020

what I meant is that in so the extra args in the makefile could be passed to luarocks via an override so you dont need a manual one a priori.

@doronbehar
Copy link
Contributor

extra args in the makefile could be passed to luarocks via an override so you dont need a manual one a priori.

@teto please do this:

  1. Apply this diff on master:
diff --git i/pkgs/development/lua-modules/overrides.nix w/pkgs/development/lua-modules/overrides.nix
index 30be2c197a5..c88d375d5be 100644
--- i/pkgs/development/lua-modules/overrides.nix
+++ w/pkgs/development/lua-modules/overrides.nix
@@ -324,5 +324,10 @@ with super;
     nativeBuildInputs = [
       pkgs.pulseaudio pkgs.pkgconfig
     ];
+    extraVariables = {
+      INST_LIBDIR = "${placeholder "out"}/lib/lua/${lua.luaversion}";
+      INST_LUADIR = "${placeholder "out"}/share/lua/${lua.luaversion}";
+      LUA_BINDIR = "${placeholder "out"}/bin";
+    };
   });
 }
  1. Then run:
ldd $(nix-build -A luajitPackages.pulseaudio)/lib/lua/5.1/pulseaudio.so
  1. Then checkout lua.pkgs.pulseaudio: Move from generated to lua-packages.nix #89767 and run again:
ldd $(nix-build -A luajitPackages.pulseaudio)/lib/lua/5.1/pulseaudio.so

And you should be able to see that pulseaudio.so is not linked almost at all in the (2) case while it should be linked in (3).

@teto
Copy link
Member

teto commented Jun 8, 2020

cool that you tried ! sry if I dont answer quickly I am pretty swamped at the moment. Maybe it would accept the extra varialbes if https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/interpreters/lua-5/build-lua-package.nix#L228 was changed to $LUAROCKS make --deps-mode=all --tree=$out ''${rockspecFilename} $PULSEAUDIO_EXTRA_VARIABLES .... ? I think there is sthg related to that in the luarocks make builder.

@doronbehar
Copy link
Contributor

doronbehar commented Jun 8, 2020

was changed to $LUAROCKS make --deps-mode=all --tree=$out ''${rockspecFilename} $PULSEAUDIO_EXTRA_VARIABLES ....

I don't understand - I never heard of this env variable before - $PULSEAUDIO_EXTRA_VARIABLES.

Now I notice, that also when I run luarocks make inside the lua-pulseaudio repo, ldd shows that the result pulseaudio.so is not well linked. Here's the output of luarocks --verbose make --local ./pulseaudio-0.2-1.rockspec when I run it inside my lua-pulseaudio repo (EDIT): Here's a gist:

https://gist.github.com/0fadaebd60bdf6ea0540f2b8990516de

@doronbehar
Copy link
Contributor

The most interesting part is: https://gist.github.com/doronbehar/0fadaebd60bdf6ea0540f2b8990516de#file-luarocks-make-log-L45 - it's as if luajit isn't found by pkg-config....

@doronbehar
Copy link
Contributor

The warnings there remind me of luarocks/luarocks#1155 (comment) 😞 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants