Allow scripts to define their interpreter

jokeyrhyme commented 2 years ago

Howdie, thanks so much for sharing this project <3

My scripts can be found via a launcher front-end just fine (I'm using onagre)

However, if there is an error in a script or other useful output, there doesn't appear to be any feedback or log entries (I've checked journalctl -xb, I've checked the stderr/stdout from both onagre and pop-launcher)

I noticed that we don't scripts are run in a way that throws away their stderr/stdout content: https://github.com/pop-os/launcher/blob/master/plugins/src/scripts/mod.rs#L62-L64

I understand that the API for pop-launcher and its plugins is built around JSON lines in stdin/stdout, so it would break the API to allow script execution to write to stdin/stdout, but should it be okay to pipe through stderr?

Should there be a way for pop-launcher to signal to the front-end that there was an error with an activation?

Cheers!

jokeyrhyme commented 2 years ago

Just to clarify: the scripts I'm trying to run do work when I invoke them directly from my terminal, but don't do anything when invoked via onagre->pop-launcher, and it's tricky trying to figure out why

mmstick commented 2 years ago

You should be able to pipe logs to a file from your script

jokeyrhyme commented 2 years ago

As an example, here's my ~/.local/share/pop-launcher/scripts/session/session-lock.nu script:

#! /usr/bin/env nu
#
# name: Lock Screen
# description: Lock the user session
# keywords: lock screen

touch ~/session-lock
/usr/bin/systemd-cat /usr/bin/echo 'session-lock'

I ran chmod a+x ~/.local/share/pop-launcher/scripts/session/session-lock.nu and can run the script directly from my terminal and observe the "session-lock" log entry show up in journalctl --follow, and can see the new ~/session-lock file appear

But, there's no log output at all, and no new ~/session-lock, when trying to run it through onagre->pop-launcher, which is very weird because it does show up in the list of search results, which ought to mean that:

the script is in the correct place
the front-matter/comments were parsed successfully

So, whatever the problem is here, I don't believe the script is actually being executed at all, and there's no feedback from pop-launcher as to what might be the problem

mmstick commented 2 years ago

Remove the first two lines and ensure your syntax is compatible with Bash

jokeyrhyme commented 2 years ago

Wow, okay, thanks, this works

But, all of the example scripts include the sha-bang, which sort of indicates that pop-launcher executes these directly instead of choosing a hardcoded-interpreter, e.g. https://github.com/pop-os/launcher/blob/master/scripts/session/session-logout.sh

And there doesn't seem to be any documentation that specifies that the script must be compatible with bash/sh: https://github.com/pop-os/launcher#script-directories

mmstick commented 2 years ago

You can specify an interpreter, but it will default to sh if not defined.

mmstick commented 2 years ago

Your shebang doesn't work because the plugin expects it to be a path to a binary.

canadaduane commented 1 year ago

There are a couple of things that I don't understand about this issue and/or solution:

Currently, the assumption built into the interpreter is that it must use # as a comment symbol, correct? i.e. if we were to use "nodejs" as an interpreter, it would not work because we couldn't satisfy both nodejs comments as // and the scripts plugin's assumption to search for lines beginning with # to find metadata.
Is the solution "good enough" here? If I understand correctly, changing the she-bang from #!/usr/bin/env nu to #!/bin/nu (or equivalent) is what worked, because the latter is a binary (nu) while the former is a command with single arg, nu.

jokeyrhyme commented 1 year ago

It's unfortunate that we currently aren't compatible with #! /usr/bin/env bash as an entrypoint, because a significant number of people (including myself) will find recommendations like this: https://stackoverflow.com/questions/10376206/what-is-the-preferred-bash-shebang/10383546#10383546

Also, maybe this issue needs to be broken up into multiple separate ones, as I'm concerned that the renaming has lost my original intention (which I buried, my bad), which is that it is quite difficult to debug when something goes wrong with either pop-launcher or any of its plugins

I have a few proposals, and if approved then I'm happy to tinker with PRs when I have time:

we support #! /path/to/interpreter and #! /usr/bin/env interpreter forms (where the space immediately-following the sha-bang is optional)
we support a few more types of comments to help us form the "front-matter" in whatever interpreted language we are using, namely: // (JavaScript, and more) and -- (Lua)
we continue to reserve the use of stdin/stdout as part of pop-launcher's API and plugin surface, however, we change our handling of stderr so that it can be used by plugin authors to emit useful diagnostics, etc when appropriate (e.g. we provide an "error" field in stdout for delivering an error through to the front-end, and anything on stderr is tagged with the source plugin and output to journald or something)

canadaduane commented 1 year ago

we support #! /path/to/interpreter and #! /usr/bin/env interpreter forms (where the space immediately-following the sha-bang is optional)

Done in #160

pop-os / launcher

Allow scripts to define their interpreter #142