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

awesome: Make sure it compiles with luajit #71292

Merged
merged 1 commit into from Oct 21, 2019

Conversation

doronbehar
Copy link
Contributor

Motivation for this change

Say a user wishes to have a derivation of awesome that will use luajitPackages instead of the default lua52Packages, he will probably use the following overlay:

self: super:

{
  awesome = super.awesome.override {
    luaPackages = super.luajitPackages;
  };
}

But, that will lead him to the following build error:

-- Could NOT find Lua (missing: LUA_LIBRARIES) (found version "5.1.4")
CMake Error at awesomeConfig.cmake:70 (message):
  Could not find Lua.  See the error above.

  You might want to hint it using the LUA_DIR environment variable, or set
  the LUA_INCLUDE_DIR / LUA_LIBRARY CMake variables.
Call Stack (most recent call first):
  CMakeLists.txt:33 (include)


-- Configuring incomplete, errors occurred!
See also "/build/source/build/CMakeFiles/CMakeOutput.log".
builder for '/nix/store/9lm5fh1ij4rxh2qmi1c5q9a3v8hmk145-awesome-4.3.drv' failed with exit code 1
error: build of '/nix/store/9lm5fh1ij4rxh2qmi1c5q9a3v8hmk145-awesome-4.3.drv' failed

Applying this patch fixes this error and it will allow to easily build a working derivation of Awesome that uses LuaJIT.

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 nix-review --run "nix-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.
Notify maintainers

cc @lovek323
cc @rasendubi
cc @ndowens

@teto
Copy link
Member

teto commented Oct 17, 2019

I feel like the best way to do this would be to pass as buildInput a luaEnv equal to lua.withPackages(p: with p; [ lgi ... ]) (and remove the with luaPackages)

@doronbehar
Copy link
Contributor Author

Furthermore, it would be even better to have a awesome-unwrapped and wrapper packages, just like with Neovim and support for extraLuaPackages just like Neovim's extraPythonPackages. Would you like me to work on that @teto?

@teto
Copy link
Member

teto commented Oct 18, 2019

Do you know about the module ? lua modules in nixos/modules/services/x11/window-managers/awesome.nix via luaModules. The wrapper approach may be more straightforward for certain aspects than the module one so it could still be an interesting thing to do.

I have no strong opinon there, I dont use awesome.

@doronbehar
Copy link
Contributor Author

Personally, I like installing most of my general use software to my user environment and not configuring it via the module systems.

Copy link
Member

@rasendubi rasendubi left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@doronbehar
Copy link
Contributor Author

@rasendubi what do you think about the suggestion to have a wrapped and unwrapped version of awesome and that the wrapped version will enable specifying lua packages to include in the search path? Just like with Neovim, wrapped and unwrapped:

{ stdenv, fetchFromGitHub, cmake, gettext, msgpack, libtermkey, libiconv
, libuv, lua, ncurses, pkgconfig
, unibilium, xsel, gperf
, libvterm-neovim
, withJemalloc ? true, jemalloc
, glibcLocales ? null, procps ? null
# now defaults to false because some tests can be flaky (clipboard etc)
, doCheck ? false
}:
with stdenv.lib;
let
neovimLuaEnv = lua.withPackages(ps:
(with ps; [ lpeg luabitop mpack ]
++ optionals doCheck [
nvim-client luv coxpcall busted luafilesystem penlight inspect
]
));
in
stdenv.mkDerivation rec {
pname = "neovim-unwrapped";
version = "0.4.2";
src = fetchFromGitHub {

{ stdenv, lib, makeWrapper
, vimUtils
, bundlerEnv, ruby
, nodejs
, nodePackages
, pythonPackages
, python3Packages
}:
with stdenv.lib;
neovim:
let
wrapper = {
extraMakeWrapperArgs ? ""
, withPython ? true, extraPythonPackages ? (_: []) /* the function you would have passed to python.withPackages */
, withPython3 ? true, extraPython3Packages ? (_: []) /* the function you would have passed to python.withPackages */
, withNodeJs? false
, withRuby ? true
, vimAlias ? false
, viAlias ? false
, configure ? {}
}:
let
rubyEnv = bundlerEnv {
name = "neovim-ruby-env";
gemdir = ./ruby_provider;
postBuild = ''
ln -sf ${ruby}/bin/* $out/bin
'';
};
/* for compatibility with passing extraPythonPackages as a list; added 2018-07-11 */
compatFun = funOrList: (if builtins.isList funOrList then
(_: lib.warn "passing a list as extraPythonPackages to the neovim wrapper is deprecated, pass a function as to python.withPackages instead" funOrList)
else funOrList);
extraPythonPackagesFun = compatFun extraPythonPackages;
extraPython3PackagesFun = compatFun extraPython3Packages;
requiredPlugins = vimUtils.requiredPlugins configure;
getDeps = attrname: map (plugin: plugin.${attrname} or (_:[]));
pluginPythonPackages = getDeps "pythonDependencies" requiredPlugins;
pythonEnv = pythonPackages.python.withPackages(ps:
[ ps.pynvim ]
++ (extraPythonPackagesFun ps)
++ (concatMap (f: f ps) pluginPythonPackages));
pluginPython3Packages = getDeps "python3Dependencies" requiredPlugins;
python3Env = python3Packages.python.withPackages (ps:
[ ps.pynvim ]
++ (extraPython3PackagesFun ps)
++ (concatMap (f: f ps) pluginPython3Packages));
binPath = makeBinPath (optionals withRuby [rubyEnv] ++ optionals withNodeJs [nodejs]);
in
stdenv.mkDerivation {

@rasendubi
Copy link
Member

As long as we keep the awesome module API stable, I'm fine with the wrapper approach. As a bonus, it could remove a minor duplication between nixpkgs awesome module and home-manager's one.

@doronbehar
Copy link
Contributor Author

This will certainly enable removing the duplication but it will require changes to the Nixpkgs' module and home-manager's module similarly. Taking this into consideration, I think it'll be wiser to tackle this proposal in a separate PR. I don't use home-manager but I think my change won't break the way home-manager adds Lua modules to the search path.

@doronbehar
Copy link
Contributor Author

@ndowens and @lovek323 are still invited for review...

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/61

@rasendubi rasendubi merged commit 2695914 into NixOS:master Oct 21, 2019
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/awesomewm-luamodules-apparently-not-taking-effect/8507/5

@doronbehar doronbehar deleted the awesome-luajit branch March 2, 2023 10:35
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

4 participants