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

remove potentially harmful LUA_PATH override #144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bb010g
Copy link
Collaborator

@bb010g bb010g commented Jan 13, 2025

i wrote this patch during the last time i was hacking on Alicorn, but hadn't submitted it then.

@bb010g bb010g force-pushed the topic/flake-lua-path branch from 76490f0 to cc10659 Compare January 13, 2025 15:55
@AstroSnail
Copy link
Contributor

I added a similar override to the flake check that runs all the lua tests in 2beba7e part of #100. It helps with test-pretty-print which, as currently written, requires both tap (which we pull in as a lit package in deps) and luaunit (which we pull in as part of the luajit package). Should this be changed?

@bb010g
Copy link
Collaborator Author

bb010g commented Jan 13, 2025

@AstroSnail I don't think setting the LUA_PATH like that is a good idea in general. I'd probably have a Lua module (//deps/init.lua?) that can be loaded that augments package.path & package.cpath as necessary, and figure out some way to require that early. luvit is a different Lua than luajit and should have its own equivalent to .withPackages to use.

@bb010g bb010g force-pushed the topic/flake-lua-path branch from cc10659 to 492ba36 Compare January 19, 2025 10:44
@LunNova
Copy link
Contributor

LunNova commented Jan 19, 2025

This was initially added in d176fbd
Should confirm dropping it hasn't broken runtest in dev shell / fix that if needed before merging.

@LunNova
Copy link
Contributor

LunNova commented Jan 19, 2025

Running luvit runtest.lua and luajit runtest.lua in devShell for this doesn't immediately explode with a dep issue.

@AstroSnail
Copy link
Contributor

i dug a bit deeper and found that it's related to the profiler fc7c860

This behavior should be accomplished by wrapping a specific Lua interpreter.

Signed-off-by: Dusk Banks <[email protected]>
@bb010g bb010g force-pushed the topic/flake-lua-path branch from 492ba36 to bbd7767 Compare February 2, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants