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

buildLuarocksPackage: accept structured luarocks config #288669

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

teto
Copy link
Member

@teto teto commented Feb 13, 2024

There is an arbitrary mapping being done right now between nixpkgs lua infrastructre and luarocks config schema. This is confusing if you use lua so let's make it possible to use the lua names in the nixpkgs, thanks to the lib.generators.toLua convertor.

The only nixpkgs thing to remember should be to put the config into luarocksConfig

buildLuarocksPackage.extraVariables should become buildLuarocksPackage.luarocksConfig.variables

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@mrcjkb
Copy link
Member

mrcjkb commented Feb 15, 2024

question: With such a large amount of rebuilds, shouldn't this go into staging?

@teto teto marked this pull request as draft February 15, 2024 21:49
@teto teto marked this pull request as ready for review February 15, 2024 21:50
@teto teto marked this pull request as draft February 15, 2024 21:50
@teto teto changed the base branch from master to staging February 15, 2024 21:50
There is an arbitrary mapping being done right now between
nixpkgs lua infrastructre and luarocks config schema.
This is confusing if you use lua so let's make it possible to use the
lua names in the nixpkgs, thanks to the lib.generators.toLua convertor.

The only nixpkgs thing to remember should be to put the config into `luarocksConfig`

`buildLuarocksPackage.extraVariables` should become `buildLuarocksPackage.luarocksConfig.variables`
@teto teto marked this pull request as ready for review February 15, 2024 22:29
@teto teto merged commit 50e877e into NixOS:staging Feb 17, 2024
23 checks passed
@teto teto deleted the structured-luarocks-config branch February 17, 2024 14:58
@Izorkin
Copy link
Contributor

Izorkin commented Mar 29, 2024

@teto this PR breaks build of luadbi-mysql package:

lua5.2-luadbi-mysql> structuredAttrs is enabled
lua5.2-luadbi-mysql> Running phase: unpackPhase
lua5.2-luadbi-mysql> unpacking source archive /nix/store/npzmjakmxhqyshmr1m33yvb0y6faa9lf-luadbi-73a234c
lua5.2-luadbi-mysql> source root is luadbi-73a234c
lua5.2-luadbi-mysql> Running phase: patchPhase
lua5.2-luadbi-mysql> Running phase: updateAutotoolsGnuConfigScriptsPhase
lua5.2-luadbi-mysql> Running phase: configurePhase
lua5.2-luadbi-mysql> Running phase: buildPhase
lua5.2-luadbi-mysql> Running phase: installPhase
lua5.2-luadbi-mysql> Missing dependencies for luadbi-mysql 0.7.2-1:
lua5.2-luadbi-mysql>    luadbi 0.7.2 (not installed)
lua5.2-luadbi-mysql> luadbi-mysql 0.7.2-1 depends on lua >= 5.1, < 5.4 (5.2-1 provided by VM: success)
lua5.2-luadbi-mysql> luadbi-mysql 0.7.2-1 depends on luadbi 0.7.2 (not installed)
lua5.2-luadbi-mysql> mkdir: cannot create directory '/homeless-shelter': Permission denied
lua5.2-luadbi-mysql> Warning: Failed searching manifest: Failed downloading https://luarocks.org/manifest-5.2 - failed downloading https://luarocks.org/manifest-5.2
lua5.2-luadbi-mysql> Warning: Failed searching manifest: Failed downloading https://raw.githubusercontent.com/rocks-moonscript-org/moonrocks-mirror/master/manifest-5.2 - failed downloading https://raw.githubusercontent.com/rocks-moonscript-org/moonrocks-mirror/master/manifest-5.2
lua5.2-luadbi-mysql> Warning: Failed searching manifest: Failed downloading https://loadk.com/luarocks/manifest-5.2 - failed downloading https://loadk.com/luarocks/manifest-5.2
lua5.2-luadbi-mysql> Error: Could not satisfy dependency luadbi 0.7.2: No results matching query were found for Lua 5.2.

@teto
Copy link
Member Author

teto commented Mar 29, 2024

the override was erasing the existing value, best to do a recursiveUpdate. As a quickfix you can do

diff --git a/pkgs/development/lua-modules/overrides.nix b/pkgs/development/lua-modules/overrides.nix
index 722c3d4940f0..7062fcbe404c 100644
--- a/pkgs/development/lua-modules/overrides.nix
+++ b/pkgs/development/lua-modules/overrides.nix
@@ -353,11 +353,13 @@ in
   });
 
   luadbi-mysql = prev.luadbi-mysql.overrideAttrs (oa: {
-    luarocksConfig.variables = {
-      # Can't just be /include and /lib, unfortunately needs the trailing 'mysql'
-      MYSQL_INCDIR = "${libmysqlclient.dev}/include/mysql";
-      MYSQL_LIBDIR = "${libmysqlclient}/lib/mysql";
-    };
+    luarocksConfig = oa.luarocksConfig // {
+        variables = {
+          # Can't just be /include and /lib, unfortunately needs the trailing 'mysql'
+          MYSQL_INCDIR = "${libmysqlclient.dev}/include/mysql";
+          MYSQL_LIBDIR = "${libmysqlclient}/lib/mysql";
+        };
+      };
     buildInputs = oa.buildInputs ++ [
       mariadb.client
       libmysqlclient

@Izorkin
Copy link
Contributor

Izorkin commented Mar 30, 2024

Thanks, patch works.

Petingoso added a commit to Petingoso/nixpkgs that referenced this pull request May 4, 2024
As this (issue)[NixOS#295022] complains, it wasn't
finding symbols properly.

I traced it to the (migration)[NixOS#288669] from buildLuarocksPackage.extraVariables to
buildLuarocksPackage.luarocksConfig.variables where someone forgot to
add .variables.

It now launches the dialogue properly.
teto pushed a commit that referenced this pull request May 9, 2024
As this (issue)[#295022] complains, it wasn't
finding symbols properly.

I traced it to the (migration)[#288669] from buildLuarocksPackage.extraVariables to
buildLuarocksPackage.luarocksConfig.variables where someone forgot to
add .variables.

It now launches the dialogue properly.
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.

3 participants