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 for sile #78565

Closed
wants to merge 2 commits into from
Closed

Lua for sile #78565

wants to merge 2 commits into from

Conversation

teto
Copy link
Member

@teto teto commented Jan 26, 2020

Motivation for this change

forgot to add cassowary in the previous PR.
Also tried to move pulseaudio to the conventional package. @doronbehar could you check I haven't broken the package, there is a warning aobut a missing folder

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.

@@ -294,6 +294,9 @@ with super;
};
});

pulseaudio = super.pulseaudio.override({
nativeBuildInputs = [ pkgs.pulseaudio pkgs.pkg-config ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teto sorry it took me a while to respond, I've been busy but I never forgot :). As for my baby pulseaudio Lua module. According to my tests, it wasn't built right - I get the following error when trying to require it:

/nix/store/9mfzwhpbjqz2b6l8vvmcflg6fhdjshzb-luajit-2.1.0-beta3/bin/lua: error loading module 'pulseaudio' from file 'pulseaudio.so':
        pulseaudio.so: cannot open shared object file: No such file or directory
stack traceback:
        [C]: at 0x0045c7e0
        [C]: in function 'require'
        (command line):1: in main chunk
        [C]: at 0x004061b0

I was wondering, why didn't you add to the overrides the make filags?, I mean these:

Suggested change
nativeBuildInputs = [ pkgs.pulseaudio pkgs.pkg-config ];
nativeBuildInputs = [ pkgs.pulseaudio pkgs.pkg-config ];
makeFlags = [
"INST_LIBDIR=${placeholder "out"}/lib/lua/${lua.luaversion}"
"INST_LUADIR=${placeholder "out"}/share/lua/${lua.luaversion}"
"LUA_BINDIR=${placeholder "out"}/bin"
];
preBuild = ''
mkdir -p ${placeholder "out"}/lib/lua/${lua.luaversion}
'';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like the luarocks bump broke some things for me as it stopped building with Warning: unmatched variable LUA_LIBDIR (and it used to at least build). The makeFlags should not be necessary as the lua infra should take care of it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out my comment here: luarocks/luarocks#1155 (comment) . I've tested compilation of libluv on Nixpkgs with Luarocks 3.2.1 and I still had the same warnings and errors, it's just that they haven't broke the build. Waiting for upstream to respond.

@doronbehar
Copy link
Contributor

@teto along with fixing the conflict, there's an interesting package I think we could put in luarocks-packages.csv: https://github.com/Koihik/LuaFormatter

Could you please try to add it and I'll test it?

@teto
Copy link
Member Author

teto commented Feb 13, 2020

I've had a go at the package but it seems huge (42 MB), to compile tons of stuff with some difficulties.

    file cannot create directory: /var/empty/local/bin.  Maybe need
    administrative privileges.

Sorry I do not have time for this at the moment but I might give it a try once it goes out of dev mode.

@doronbehar
Copy link
Contributor

O.K @teto no big deal. In general, what is the best way to make a PR with a new Luarock based package? I once tried to add a line in luarocks-packages.csv for a package I wanted to add and I ran the update script but it updated so many more packages that I hadn't had the nerve to put all of these changes in my PR.

@teto
Copy link
Member Author

teto commented Feb 13, 2020

@doronbehar that's how you describe. Just at the end don't commit everything but use git add -p to only commit the changes you are interested. You can discard the rest via git reset --hard or git stash

@doronbehar
Copy link
Contributor

I see. And what if we'd like to update lua.pkgs.libluv for the sake of maybe fixing #79870, how would you go and do that?

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.

2 participants