[Bug]: Incomplete fish support

[pcrockett]

Current Behavior (bug)

Devbox makes a good attempt at supporting fish, but it's not complete enough to work. For example, it generates a fish config in /tmp but then in the config it tries to source a traditional shell script:

# in /tmp/devbox614133565/config.fish

# Source the hooks file, which contains the project's init hooks and plugin hooks.
source /home/user/some/path/.devbox/gen/scripts/.hooks.sh

This is kind of like... a bash script trying to source a python script. You can see where this falls down when you try to use the poetry package. Devbox shows this error in fish after running devbox shell:

Starting a devbox shell...
~/some/path/.devbox/gen/scripts/.hooks.sh (line 1): $(...) is not supported. In fish, please use '(command)'.
poetry env use $(command -v python) --no-interaction
               ^
from sourcing file ~/Code/Terminus/.devbox/gen/scripts/.hooks.sh
    called on line 162 of file /tmp/devbox614133565/config.fish
from sourcing file /tmp/devbox614133565/config.fish
source: Error while reading file “/home/phil/Code/Terminus/.devbox/gen/scripts/.hooks.sh”

This also makes the shell environment incomplete at best, because the fish config is not fully executed to the end.

Expected Behavior (fix)

While fish is only partially supported, I would expect that the hooks are not sourced, with a warning printed to screen saying that init hooks aren't supported for fish yet.

Additional context

Version:     0.6.0
Platform:    linux_amd64
Commit:      fdbd72f230b3520dde60d72c57d9460922ade656
Commit Time: 2023-10-03T18:04:08Z
Go Version:  go1.21.1
Launcher:

{
  "packages": [
    "python@3.8",
    "poetry@latest"
  ],
  "shell": {
    "init_hook": [
      "poetry install"
    ],
    "scripts": {
    }
  }
}

[savil]

Thanks for reporting this @pcrockett. This is definitely an area we can improve upon.

In the current design, the init-hooks run in both the user's shell, as well as the devbox-scripts which run in sh shells. For most users or teams, init-hooks should be written as posix standard scripts, since those are supported by most shells.

However, for users or teams that also use fish these init-hooks need to be written to support both posix and fish scripts.

The specific error you are running into is that the init-hooks defined in our plugins/ folder are written using just posix. They should be fixed to also support fish shells.

[savil]

@pcrockett which version of fish shell are you using?

[savil]

I'm on version 3.6.1, and am able to execute that line inserted by the poetry plugin:

> poetry env use $(command -v python) --no-interaction
Using virtualenv: /Users/savil/code/jetpack/devbox/examples/development/python/poetry/poetry-demo/.venv

[savil]

it looks like fish supports the $(cmd) command substitution syntax as of version 3.4.0 from March 2022 https://fishshell.com/docs/current/relnotes.html#fish-3-4-0-released-march-12-2022

[pcrockett]

I'm not at my laptop now, but I'm running the version of fish that comes from Ubuntu package repos (which I know is fairly outdated).

That does make a lot of sense; I guess compatibility with various shells could be considered a plugin concern. I'm hopeful that not many plugin authors will need to worry about it, otherwise it could become a significant problem for users of less popular shells.

Would you be open to a PR that improves poetry plugin support for an old fish? I'm not making any commitments, but I could see myself digging into it some time...

Oct 10, 2023 23:04:46 savil @.***>:

it looks like fish supports the $(cmd) command substitution syntax as of version 3.4.0 from March 2022 https://fishshell.com/docs/current/relnotes.html#fish-3-4-0-released-march-12-2022

— Reply to this email directly, view it on GitHub[https://github.com/jetpack-io/devbox/issues/1550#issuecomment-1756256478], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAIQCEOCO67HRFXAE4JVRLLX6WZ63AVCNFSM6AAAAAA524Y4YOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJWGI2TMNBXHA]. You are receiving this because you were mentioned. [Tracking image][https://github.com/notifications/beacon/AAIQCENWOZE2ILLH6IBFOQDX6WZ63A5CNFSM6AAAAAA524Y4YOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTIVZMN4.gif]

[Lagoja]

Definitely open to contributions if you have time to dig in

[savil]

@pcrockett The PR fix auto-closed the issue. We still recognize there's an underlying issue around portability for init-hooks, which we'll keep in mind and seek to address. Thanks for raising the issue!