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

Cannot launch start SillyTavern with Extras #29

Open
ostr00000 opened this issue Dec 28, 2023 · 2 comments
Open

Cannot launch start SillyTavern with Extras #29

ostr00000 opened this issue Dec 28, 2023 · 2 comments
Assignees
Labels
🐛Bug [ISSUE] Ticket describing something that isn't working 🧑‍💻 In Progress [ISSUE] Assigned to an issue that is curretly being worked on ❕ Medium Priority [ISSUE] Needs to be dealt with at some point

Comments

@ostr00000
Copy link

OS: Linux (Ubuntu 23.10)

  1. I believe that this code is incorrect:

    exec "$detected_terminal" -e "cd $(dirname "$0")./SillyTavern && ./start.sh" &
    exec "$detected_terminal" -e "cd $(dirname "$0")./SillyTavern-extras && ./start.sh" &

    According to bash manual, exec will replace current shell with specified subcommand.
    This will only launch the first of these commands.

  2. Not sure about all the supported terminals, but in case of konsole there is an error: "Could not find binary: " "cd".
    This is probably because konsole try to run specified command exactly as a new program. There is also used bash syntax (&&), so it should be wrapped in bash caommnad.

  3. Also, probably the path is wrong. In my case, this command is run: konsole -e 'bash -c '\''cd [...]/SillyTavern-Launcher./SillyTavern && ./start.sh'\''' (Note unnecessary . character after Launcher word).

I used this code as a workaround:

            "$detected_terminal" -e "bash -c 'cd $(dirname "$0")/SillyTavern && ./start.sh'" &
            "$detected_terminal" -e "bash -c 'cd $(dirname "$0")/SillyTavern-extras && ./start.sh'" &
  1. In SillyTavern-extras project, there are missing execute permissions for start.sh file. This may be fixed by launcher or maybe bettor to report and fix in that project?

  2. Script start.sh in SillyTavern-extras should use exactly the same conda environment which was configured in launcher (install.sh). According to code in https://github.com/SillyTavern/SillyTavern-Extras/blob/main/start.sh this is configured by setting $CONDA_PATH environment variable. This should be done in laucher.sh.

@Frityet
Copy link

Frityet commented Jan 1, 2024

this is configured by setting $CONDA_PATH environment variable. This should be done in laucher.sh.

I had considered this while rewriting the conda finding code, but the issue is that I am not sure if $CONDA_PATH is something that is already used/standard and so would be detected with other programmes, so having the installer have a side effect of setting that could be malicious

@ostr00000
Copy link
Author

having the installer have a side effect of setting that could be malicious

That reasonable concern.

But from install.sh I deduced that CONDA_PATH variable is mandatory (otherwise if variable is not set, there are errors, see #28).

  1. If this is indeed mandatory in install script, then it should be also used in start.sh.
  2. Otherwise (if this is an option for user, to customize script and use specified conda), then install.sh needs to be refactored a bit, to check if conda is used either from:
    a) environment variable,
    b) detected existing conda installation by script,
    c) miniconda installed locally by install.sh script (this is my case, I do not have conda installed neither for system nor for user).

This is how I image a perfect launcher: "run install.sh, then start.sh". At this moment, support for 2.c) is missing.


Or maybe README.md needs a little adjustment?
I think that this may work: "If conda is not detected by install script, you must set environment variable CONDA_PATH pointing to existing conda installation. If conda is not installed at all, you must set CONDA_PATH pointing to directory where conda installation will be placed".
But I personally prefer this to be done with minimal user interaction. For example: conda installation may have default path [...]/SillyTavern-Launcher/miniconda/, and if this is dir is detected, then this conda is used for SillyTavern launchers. What you think about that solution?

@deffcolony deffcolony added ❕ Medium Priority [ISSUE] Needs to be dealt with at some point 🐛Bug [ISSUE] Ticket describing something that isn't working 🧑‍💻 In Progress [ISSUE] Assigned to an issue that is curretly being worked on labels Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛Bug [ISSUE] Ticket describing something that isn't working 🧑‍💻 In Progress [ISSUE] Assigned to an issue that is curretly being worked on ❕ Medium Priority [ISSUE] Needs to be dealt with at some point
Projects
None yet
Development

No branches or pull requests

3 participants