lua: add withPackages function #54460

teto · 2019-01-22T11:02:53Z

First step towards more automation similar to the haskell backend.
Extracted from #33903 to make it easier to review.

Motivation for this change

I want to be able to create lua environments with lua.withPackages().

If some things appear awkward, more context at #33903 can help or just ask and I will try to answer if I can. I started this 1 year and half ago with no nix knowledge so some things can be improved (I would like to have a lua lib component as haskell does, my shell hooks might not be the best) but it kinda works.

Something I did not dare do is merge the three lua 5.1/ lua 5.2 /lua5.3 interpreters into one which would remove some duplication. If I have the greenlight I want to do it.

You can test with sthg like:
NIX_PATH="nixpkgs=/home/teto/nixpkgs2:$NIX_PATH" nix-shell -p 'lua5_3.withPackages(ps: [ ps.lpeg ps.cqueues ])'

Things done

7c6f434c · 2019-01-22T11:09:06Z

Please unbreak LuaJIT evaluation back…

teto · 2019-01-23T09:15:08Z

So I am running locally the command from ofborg's readme with -vvvvv in order to diagnose the issue

$ curl -o outpaths.nix https://raw.githubusercontent.com/NixOS/ofborg/released/ofborg/src/outpaths.nix
$ GC_INITIAL_HEAP_SIZE=4g nix-env -f ./outpaths.nix -qaP --no-name --out-path --arg checkMeta true -vvvvv > out-paths

but it fails with

instantiated 'source' -> '/nix/store/25pdni30v75m9b85dygai4yvw4ydc3h4-source.drv'
processing attribute 'stdenv'
processing attribute 'strictDeps'
processing attribute 'system'
processing attribute 'userHook'
instantiated '9pfs-20150918' -> '/nix/store/4h2sk33qpk3hjap8mkjr0j1nixg0ynr6-9pfs-20150918.drv'
evaluating attribute '__splicedPackages'
error: out of memory

@grahamc any idea on how to properly debug what part of the code is responsible for the error (like tracing the name of the function etc.)

teto · 2019-01-24T07:55:29Z

ok found the problem @7c6f434c could you have a look at the overall PR please? The shell script might need some changes/cleanup. I think I should rebase it on staging (what is staging-next ?).

7c6f434c · 2019-01-24T09:31:33Z

pkgs/development/interpreters/lua-5/setup-hook.sh

+    cd -
+}
+
+addEnvHooks "$hostOffset" startLuaEnvHook


Won't this add paths without checking in luaPathsSeen?

luaPathsSeen is effective when using the wrapper. The setup-hook is a bit different. With a naive version adding every possible folder for any package, it could end up too long. The version thus checks if the folder exists and only in that case it will add it to LUA_PATH. The "requiredLuaModules" calls unique function so it should prevent duplicates from appearing.

Are these called for requiredLuaModules or for buildInputs, though?

(Also, is there a reasonable way to just use wrappers during normal builds, to reduce code duplication, as well as confusion because of inexact duplication?)

(also, if luaPathsSeen is not relevant for this file, why the comment mentions it?)

Do you know if any duplication happens in case of multiple buildInputs propagate the same path?

I've added a check against duplicates but this hook is run when building user environments so IIRC it is run only on the environment folder.

7c6f434c · 2019-01-24T09:35:42Z

Re: target branch — I think if building lua*Packages and torch*Packages takes reasonable time locally, it can go to master; otherwise yes, staging is better.

teto · 2019-01-24T12:42:33Z

lua and torch packages are usually fast to compile so master should be fine IMO. Rest assured that should any problem arise I will promptly fix them as I would like to follow up with other PRs as I described in the first updated post of #33903

First step towards more automation similar to the haskell backend. Follow up of NixOS#33903

teto · 2019-01-30T07:34:07Z

I removed the luaWrap for now as it doesn't look necessary for now, and what I want to do first is to update the packages so that they use lua environments. It will make my other changes easier to rebase/submit.
Could you have another look please ?

7c6f434c · 2019-01-30T12:08:11Z

Nice, GitHub shows me the list of changes including a file that has empty diff. What the… convenient and consistent UI.

I like the current version much better; I am not sure if handling repeated buildInputs in the shell code is worth the effort (maybe we could just see how it goes).

Do I understand correctly that you expect this version of interpreter packages to stick for a while?

I guess if this part is stable, I could reread it once more and merge it…

check for duplicate as well.

teto · 2019-01-30T12:31:34Z

Do I understand correctly that you expect this version of interpreter packages to stick for a while?

Apart from merging the lua5.1/5.2/5.3 interpreters into one - which is low priority for me - my next steps would be:
1/ to make some packages (neovim/mpv/awesome mostly) depend on withPackages.
2/ Introduce buildLuarocksPackage and the "generated" packages ecosystem. I had first intended to completely replace the current infrastructure but in #33903, I was advised to deploy my changes under a different name to ease upstreaming.

In terms of building, I will need to reintroduce a luaWrap (needed by some programs) but having the withPackages already available in nixos-unstable makes developement easier (and upgrade of some packages possible) so I am looking forward to this being merged.

7c6f434c · 2019-01-30T12:37:22Z

pkgs/development/interpreters/lua-5/setup-hook.sh

+    # export only if the folder exists else LUA_PATH grows too big
+    if  [ ! -d "$topDir" ]; then return; fi
+    # check for duplicate
+    if [[ $LUA_PATH = *"$pattern"* ]]; then return; fi


This check requries that patterns cannot be substrings of each other (which is true; not sure if worth a comment); but shouldn't either the variable to check depend on varName or ;$LUA_PATH just be checked against ;$topDir/ ?

right it should be topDir/$pattern, let me fix it, At the same time, I never had any trouble with duplicate buildInputs so I am tempted to just remove the line.

ha no sorry pattern already contains the basefolder so it should be ok as is. I got confused because pattern don't have the same meaning in addToLuaSearchPathWithCustomDelimiter (absolute path) and addToLuaPath (relative path).

I am OK with dropping.

What I meant is that if ;$LUA_PATH contains ;$topDir we know that the bsae directory has been processed already.

ok I've dropped the check and renamed pattern to abs(olute)Pattern to make it easier it has a different meaning. No need to troll myself.

checking just against topdir would not be enough since several patterns are checked against the same topdir. The first matching pattern would discard others while they may be valid as well.

also makes clearer that in one case pattern is a full path while in the other it is relative.

7c6f434c · 2019-01-30T14:01:08Z

@GrahamcOfBorg build love_11 luaPackages.luarocks luaPackages.luarocks-nix luaPackages.http

7c6f434c · 2019-01-30T14:14:43Z

GitHub: «10 checks passed» — that's counting only eval checks after I intentionally waited for a build to pass. Sigh.

vcunat · 2019-02-06T15:30:37Z

Cross-ref: #55345

teto force-pushed the lua_step1 branch from eee28c3 to 916a57d Compare January 23, 2019 02:18

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ labels Jan 24, 2019

teto mentioned this pull request Jan 24, 2019

[RFC] get lua support closer to python #33903

Closed

21 tasks

7c6f434c reviewed Jan 24, 2019

View reviewed changes

teto force-pushed the lua_step1 branch from f6c165c to e17adc9 Compare January 24, 2019 10:54

lua: add withPackages function

ca16a5e

First step towards more automation similar to the haskell backend. Follow up of NixOS#33903

teto force-pushed the lua_step1 branch from e17adc9 to ca16a5e Compare January 30, 2019 07:04

lua: simplify setup hook in wrapper.nix

df7a0d5

check for duplicate as well.

7c6f434c reviewed Jan 30, 2019

View reviewed changes

lua setup hook: dropping the check for duplicate

5aab7e6

also makes clearer that in one case pattern is a full path while in the other it is relative.

7c6f434c merged commit c4519cf into NixOS:master Jan 30, 2019

teto deleted the lua_step1 branch January 30, 2019 14:49

dtzWill mentioned this pull request Jan 31, 2019

lua-5 setup-hook: quiet noisy 'cd -' printing path repeatedly #54961

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lua: add withPackages function #54460

lua: add withPackages function #54460

teto commented Jan 22, 2019 •

edited

7c6f434c commented Jan 22, 2019

teto commented Jan 23, 2019

teto commented Jan 24, 2019

7c6f434c Jan 24, 2019

teto Jan 24, 2019

7c6f434c Jan 24, 2019

7c6f434c Jan 30, 2019

teto Jan 30, 2019

7c6f434c commented Jan 24, 2019

teto commented Jan 24, 2019

teto commented Jan 30, 2019

7c6f434c commented Jan 30, 2019

teto commented Jan 30, 2019

7c6f434c Jan 30, 2019

teto Jan 30, 2019

teto Jan 30, 2019

7c6f434c Jan 30, 2019

7c6f434c Jan 30, 2019

teto Jan 30, 2019

teto Jan 30, 2019

7c6f434c commented Jan 30, 2019

7c6f434c commented Jan 30, 2019

vcunat commented Feb 6, 2019

lua: add withPackages function #54460

lua: add withPackages function #54460

Conversation

teto commented Jan 22, 2019 • edited

Motivation for this change

Things done

7c6f434c commented Jan 22, 2019

teto commented Jan 23, 2019

teto commented Jan 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

7c6f434c commented Jan 24, 2019

teto commented Jan 24, 2019

teto commented Jan 30, 2019

7c6f434c commented Jan 30, 2019

teto commented Jan 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

7c6f434c commented Jan 30, 2019

7c6f434c commented Jan 30, 2019

vcunat commented Feb 6, 2019

teto commented Jan 22, 2019 •

edited