Shell detection for luarocks fails on edge case: fish as an interactive only shell

[sandersantema]

I've setup my fish shell as an interactive only shell, which means echo $SHELL reports /bin/bash in fish when on its own and /bin/sh from tmux. fish is launched automatically from bash, bash is accessed by using bash --norc. I can't remember exactly why I've done this but I believe setting fish as the login shell had some nasty consequences on my OpenSUSE Tumbleweed system.

All of this leads to activate_hererocks_cmd from packer.luarocks reporting the wrong shell, after which hererocks tries to use the wrong activate binary, namely activate instead of activate.fish. Such as can be seen in the following log message:

[ERROR Mon 15 Feb 2021 07:48:31 PM CET] ...site/pack/packer/opt/packer.nvim/lua/packer/luarocks.lua:456: Failed to install packages { \"luaposix\" }: {\
  data = {\
    exit_code = 2,\
    signal = 0\
  },\
  msg = \"Error running luarocks install luaposix\",\
  output = {\
    data = {\
      stderr = { \"/home/sandersantema/.cache/nvim/packer_hererocks/2.1.0-beta3/bin/activate: line 1: declare: `deactivate-lua': not a valid identifier\", \"/home/sandersantema/.cache/nvim/packer_hererocks/2.1.0-beta3/bin/activate: line 17: `deactivate-lua': not a valid identifier\" },\
      stdout = {}\
    },\
    err = {\
      stderr = {},\
      stdout = {}\
    }\
  }\
}

One possible fix which I can think of is checking whether vim.o.shell is set and if so setting the shell to that value and if not setting it to os.getenv(SHELL), like this:

local function activate_hererocks_cmd(install_path)
  local activate_file = 'activate'
  local user_shell
  if vim.o.shell == "" then
    user_shell = os.getenv('SHELL')
  else
    user_shell = vim.o.shell
  end
  local shell = user_shell:gmatch('([^/]*)$')()
  if shell == 'fish' then
    activate_file = 'activate.fish'
  elseif shell == 'csh' then
    activate_file = 'activate.csh'
  end

  return fmt('source %s', util.join_paths(install_path, 'bin', activate_file))
end

I think this makes sense given that nvim would use the wrong shell too without having vim.o.shell set. Unfortunately this causes an error:

[ERROR Mon 15 Feb 2021 08:04:37 PM CET] ...im/site/pack/packer/opt/packer.nvim/lua/packer/async.lua:17: Error in coroutine: ...site/pack/packer/opt/packer.nvim/lua/packer/luarocks.lua:135: E5560: nvim_get_option must not be called in a lua loop callback

As a workaround I can start nvim as env SHELL=fish nvim or I could permanently set $SHELL to fishin my environment which might have been the best idea all along eitherway but I suppose it still might be a nice idea to capture possible edge cases such as this.

[wbthomason]

Thanks for the report! I admit, I had not anticipated this case. Do you think it would be reasonable to cache the correct shell identity as a module-local variable, rather then checking it in activate_hererocks_cmd? That would sidestep the "can't use nvim_get_option in a callback" issue, and I don't think the value of SHELL should change while Neovim is running.

[nanotee]

I may be totally wrong here, but I wonder if the shell-specific code is necessary. It looks like the activation scripts for hererocks only add stdpath('cache') .. '/packer_hererocks/2.1.0-beta3/bin' to $PATH. Wouldn't it be possible to run the luarocks executable in packer_hererocks/2.1.0-beta3/bin/ directly without running it in a shell? (You might even be able to avoid messing with shellescape())

[wbthomason]

That's a fair idea. I thought about this when adapting neorocks to the packer luarocks module, but opted to keep using the activation script in case luarocks updates what that script does at some point in the future. Still, might be a cleaner solution.