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

Adds 'shell' command. #492

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adds 'shell' command. #492

wants to merge 2 commits into from

Conversation

ignacio
Copy link
Contributor

@ignacio ignacio commented Feb 1, 2016

Adds the 'luarocks shell' command, that launches a new shell with the environment variables LUA_PATH and LUA_CPATH set with the values configured when installing LuaRocks.

This was suggested here.

Adds the 'luarocks shell' command, that launches a new
shell with the environment variables LUA_PATH and LUA_CPATH
set with the values configured when installing LuaRocks.
-- @return boolean This function always succeeds.
function shell_cmd.run(...)
local lr_path, lr_cpath = cfg.package_paths()
local path_sep = cfg.export_path_separator
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused

@hishamhm
Copy link
Member

hishamhm commented Feb 1, 2016

I've been experimenting locally with this idea too. @Tieske suggested this could be implemented as a command, luarocks-shell. The command, if implemented by writing it during build, may be less fragile and less dynamic (pros and cons there); I haven't figured out which one is really best.

I think it also needs to set the PATH, and add not only the path of bin/ entries for the active rock trees in it, but also the path of the LuaRocks binaries itself. (i.e. it should be guaranteed that a plain luarocks invocation works inside a launched shell).

In any case, having something like luarocks-shell in my machine for a few days, I noticed it's something I ended up using a lot. It will be a very useful feature.

@ignacio
Copy link
Contributor Author

ignacio commented Feb 1, 2016

Ok, thanks both @hishamhm and @mpeterv for the pointers. I'll update this PR, but the possibility of generating a script at install time is not something I would oppose.
I'll take a stab at that but it will be later this week.

In the "cons" section of the luarocks-shell script is that having the util.remove_path_dupes functionality in Windows is going to be "fun" to implement :)

@ignacio
Copy link
Contributor Author

ignacio commented Feb 1, 2016

@hishamhm could you share the script you're using? I'm taking a look at what would be needed to generate it and while the Windows side of things is clear to me, I'm a bit lost as to what to do on the "other" side :)

@hishamhm
Copy link
Member

hishamhm commented Oct 28, 2016

This ended up falling on the wayside, sorry. My initial attempt at a stand-alone luarocks-shell for Unix was simply this:

#!/bin/sh

export PATH=$(dirname $(readlink -f "$0"))":$PATH"
eval $(luarocks path --bin)
export LUA_PATH LUA_CPATH PATH
exec "$SHELL" "$@"

This is finding the path of the binary dynamically and using luarocks path to get the variables, but it would also be possible to build this script during installation so that it would hardcode the paths (I see pros and cons in both approaches).

@Tieske
Copy link
Contributor

Tieske commented Apr 2, 2018

I like this approach (just ran into this PR for the first time, dunno how I missed it before)

If the script is part of the LR installation, then based on its own location it would be able to find everything it needs right? No need to do hardcoded install time stuff...

@hishamhm
Copy link
Member

hishamhm commented Apr 2, 2018

@Tieske Would it be possible to launch a CMD instance that auto-runs the Visual Studio setup batch file on startup then gives you an interactive shell? This would be necessary to give us a "shell that works" on Windows.

(We could also check that this Visual Studio batch file can be found during INSTALL.BAT)

@Tieske
Copy link
Contributor

Tieske commented Apr 3, 2018

That is already the case (if it could be found) see https://github.com/luarocks/luarocks/blob/master/install.bat#L960

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