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

luaPackages.cjson: move to generated #62714

Merged
merged 1 commit into from Jun 5, 2019
Merged

luaPackages.cjson: move to generated #62714

merged 1 commit into from Jun 5, 2019

Conversation

teto
Copy link
Member

@teto teto commented Jun 5, 2019

Tested with nix-review

Motivation for this change
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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@rasendubi
Copy link
Member

cjson-2.1.0.6-1 has a bug that prevents it from working with lua >= 5.2:

Note that this is a runtime error and is not caught in the process of building the package. (We can add a simple lua -e "require 'cjson'" in the checkPhase to catch this in the future.)

I haven't found a patch we could apply, so I think we should either 1) mark the package as broken for lua >= 5.2, too, or 2) downgrade the package to 2.1.0-1. (I prefer the downgrade.)

@teto
Copy link
Member Author

teto commented Jul 1, 2019

one can set a PR that pins the version by setting the version in the appropriate column of the csv file maintainers/scripts/luarocks-packages.csv .
luarocks 3 introduced better support for testing so ideally lua-cjson would update their rockspec to launch their testsuite (should be easy since they already have a testsuite).

I had started some work in luarocks-nix towards better support of checkPhase and all but lacked time and a good example. This could be it.

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

2 participants