lua: merge lua5.X interpreters #59919

teto · 2019-04-20T08:10:58Z

similar to what was done for python.
Makes it easier to change the passthru settings and the lua infrastructure.

Motivation for this change

Things done

teto · 2019-04-21T13:10:08Z

the expansion of makeFlagsArray is awkward. Seems to work locally :/

similar to what was done for python. Makes it easier to change the passthru settings and the lua infrastructure.

teto · 2019-04-26T11:15:25Z

@7c6f434c what do you think ? it's not more elegant than the previous implementation but it makes it easier to keep lua interpreters in sync (especially the passthru part with all the buildEnv logic).

7c6f434c · 2019-04-27T06:27:01Z

pkgs/development/interpreters/lua-5/interpreter.nix

+    "INSTALL_TOP=${placeholder "out"}"
+    "INSTALL_MAN=${placeholder "out"}/share/man/man1"
+    "R=${version}"
+    ''LDFLAGS="-fPIC"''


Maybe add comment why these quotes are needed (and why CFLAGS cannot be handled like that)?

7c6f434c · 2019-04-27T06:29:04Z

Seems fine to me. a grep a is a FIXME marker? Does that part work now?

7c6f434c · 2019-04-27T07:16:08Z

pkgs/development/interpreters/lua-5/default.nix

@@ -29,44 +25,36 @@ in rec {
      sed -e 's/ALL_T *= */& $(LUA_SO)/' -i src/Makefile
    '';

-    # a grep a
    postBuild = stdenv.lib.optionalString (!stdenv.isDarwin) ''
      set -x


Is there a reason to keep verbosity here?

absolutely none sorry :/

Mwahaha that's what happens when you remove FIXME without looking around why it was there! (Not that it matters)

Yep, I usually leave a TODO but the a grep a looked like a mispaste. It also happens when I work on a PR over longer than expected.
I believe it's ready to squash & merge. Not sure if it should go to staging or master though.

arf it wasn't. Damn shell expansion rules ! reverted my cosmetic-induced bug.

teto · 2019-04-27T08:06:12Z

@GrahamcOfBorg build lua5_2 lua5_2_compat lua5_3 lua5_3_compat lua5_1 luaPackages.lpeg

7c6f434c · 2019-04-27T08:16:15Z

@GrahamcOfBorg build lua5_2 lua5_2_compat lua5_3 lua5_3_compat lua5_1 luaPackages.lpeg

7c6f434c · 2019-04-27T08:17:36Z

Wait, so quoting works for CC? Would it also work for CFLAGS then?

teto · 2019-04-27T08:23:21Z

In current nixpkgs, lua5.1 has an extra hydraPlatforms = stdenv.lib.platforms.linux; compared to this PR. Should I keep it ? what is that and why only 5.1 has it; not 5.2 or 5.3 ?

7c6f434c · 2019-04-27T08:29:54Z

I would guess that in old days Darwin was even lower tier of support, so somebody wanted to remove the platforms restriction without needing to consider Hydra impact. Feel free to git blame to find out, of course.

teto · 2019-04-27T10:07:41Z

@GrahamcOfBorg build lua5_2 lua5_2_compat lua5_3 lua5_3_compat lua5_1 luaPackages.lpeg

teto · 2019-04-27T12:35:58Z

@GrahamcOfBorg build lua5_2 lua5_2_compat lua5_3 lua5_3_compat lua5_1 luaPackages.lpeg

flokli · 2019-05-20T22:10:56Z

This seems to have broken the ultrastardx build, at least according to a git bisect:

Linking ../game/ultrastardx
Warning: "crti.o" not found, this will probably cause a linking failure
Warning: "crtn.o" not found, this will probably cause a linking failure
/nix/store/2dfjlvp38xzkyylwpavnh61azi0d168b-binutils-2.31.1/bin/ld: warning: ../game/link.res contains output sections; did you forget -T?
/nix/store/2dfjlvp38xzkyylwpavnh61azi0d168b-binutils-2.31.1/bin/ld: cannot find -llua5.2
Error: Error while linking
Fatal: There were 1 errors compiling module, stopping
Fatal: Compilation aborted
Error: /nix/store/6y49zi0s1s7fwirv4yd3835khnmh0wl0-fpc-3.0.0/bin/ppcx64 returned an error exitcode
make[1]: *** [Makefile:233: ../game/ultrastardx] Error 1
make[1]: Leaving directory '/build/source/src'
make: *** [Makefile:136: all] Error 2
builder for '/nix/store/kp3i70044s81b746c733201wsjl4x8s8-ultrastardx-2017.8.0.drv' failed with exit code 2

(nix log /nix/store/kp3i70044s81b746c733201wsjl4x8s8-ultrastardx-2017.8.0.drv)
https://hydra.nixos.org/build/93748657/nixlog/1

bad commit: 672c3c1

teto · 2019-05-21T01:43:19Z

I can reproduce, I'll look into it.

teto · 2019-05-21T11:02:31Z

I think the flags are wrong, looking at lua5.2 pkgconfig, it advises to link with -llua, not -llua5.2:
Libs: -L/nix/store/l00fyl6fqhgbw2rg3nkw0qwd3jh32ibp-lua-5.2.4/lib -llua -lm Maybe the maintainer could bump the package to today's version as there seems to be activity but no release on the github https://github.com/UltraStar-Deluxe/USDX

flokli · 2019-05-22T11:59:16Z

@teto I'm not sure if I understand the problem. lua 5.2 pkgconfig did say -llua before and after 672c3c1, so this can't be the reason for the build to fail.

The ultrastardx package is already at the latest release, and there seem to be no lua-related changes in the diff.

teto · 2019-05-25T08:35:21Z

Shouldn't they use pkg-config --libs lua to get the flags instead ?

flokli · 2019-05-25T12:11:31Z

they seem to do: https://github.com/UltraStar-Deluxe/USDX/blob/114ff0c11e8f2c415404e75db213f97a744f487b/configure.ac#L196-L218

teto · 2019-05-25T15:31:53Z

I've opened an issue here UltraStar-Deluxe/USDX#462

flokli · 2019-06-02T22:41:06Z

@teto I'm not sure I understand how this PR could have broken it, and why it did work before.

GrahamcOfBorg added the 6.topic: lua label Apr 20, 2019

teto force-pushed the lua_interpreters branch 2 times, most recently from 23e4e2b to 9f890e9 Compare April 21, 2019 12:25

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

teto added 2 commits April 25, 2019 16:16

lua: merge lua5.X interpreters

4359ac6

similar to what was done for python. Makes it easier to change the passthru settings and the lua infrastructure.

only needed for 5.3

7c91c7e

teto force-pushed the lua_interpreters branch from 9f890e9 to 7c91c7e Compare April 25, 2019 08:55

fixup

7516b70

7c6f434c reviewed Apr 27, 2019

View reviewed changes

teto force-pushed the lua_interpreters branch from 84cd5a4 to 91e0ed7 Compare April 27, 2019 07:21

teto marked this pull request as ready for review April 27, 2019 08:02

cleanup

3b1d9ba

teto force-pushed the lua_interpreters branch from 91e0ed7 to 3b1d9ba Compare April 27, 2019 08:10

Business as usual, trying to accomodate darwin

002a4a3

properly expand on darwin

7055239

teto merged commit 672c3c1 into NixOS:master Apr 27, 2019

teto deleted the lua_interpreters branch April 27, 2019 13:00

lblasc mentioned this pull request Apr 30, 2019

LuaJIT: cannot override version & packages #60469

Closed

chkno mentioned this pull request Feb 1, 2020

lua5: Fix src URLs #78978

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lua: merge lua5.X interpreters #59919

lua: merge lua5.X interpreters #59919

teto commented Apr 20, 2019

teto commented Apr 21, 2019

teto commented Apr 26, 2019

7c6f434c Apr 27, 2019

7c6f434c commented Apr 27, 2019

7c6f434c Apr 27, 2019

teto Apr 27, 2019

7c6f434c Apr 27, 2019

teto Apr 27, 2019

teto Apr 27, 2019

teto commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

teto commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

teto commented Apr 27, 2019

teto commented Apr 27, 2019

flokli commented May 20, 2019 •

edited

teto commented May 21, 2019

teto commented May 21, 2019

flokli commented May 22, 2019

teto commented May 25, 2019

flokli commented May 25, 2019

teto commented May 25, 2019

flokli commented Jun 2, 2019

lua: merge lua5.X interpreters #59919

lua: merge lua5.X interpreters #59919

Conversation

teto commented Apr 20, 2019

Motivation for this change

Things done

teto commented Apr 21, 2019

teto commented Apr 26, 2019

7c6f434c Apr 27, 2019

Choose a reason for hiding this comment

7c6f434c commented Apr 27, 2019

7c6f434c Apr 27, 2019

Choose a reason for hiding this comment

teto Apr 27, 2019

Choose a reason for hiding this comment

7c6f434c Apr 27, 2019

Choose a reason for hiding this comment

teto Apr 27, 2019

Choose a reason for hiding this comment

teto Apr 27, 2019

Choose a reason for hiding this comment

teto commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

teto commented Apr 27, 2019

7c6f434c commented Apr 27, 2019

teto commented Apr 27, 2019

teto commented Apr 27, 2019

flokli commented May 20, 2019 • edited

teto commented May 21, 2019

teto commented May 21, 2019

flokli commented May 22, 2019

teto commented May 25, 2019

flokli commented May 25, 2019

teto commented May 25, 2019

flokli commented Jun 2, 2019

flokli commented May 20, 2019 •

edited