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

Function argument substitution failed #109

Open
Diamondy4 opened this issue Sep 20, 2023 · 2 comments
Open

Function argument substitution failed #109

Diamondy4 opened this issue Sep 20, 2023 · 2 comments

Comments

@Diamondy4
Copy link

Diamondy4 commented Sep 20, 2023

Trying to resholve this script.

notifyScript = resholve.writeScript "matterhorn-notify-script" {
        inputs = [jq libnotify coreutils which];
        interpreter = "${bash}/bin/bash";
      } (builtins.readFile ./notifyV2);

jq and notify-send are not substituted in has function call, leading to script failure (jq/notify-send are not found obviously). Tried to use fix but it does nothing. Currently using keep and replaceStrings as a fallback, but it's quite hacky. Is this a bug?

Invalid output
#!/nix/store/vqvj60h076bhqj6977caz0pfxs6543nb-bash-5.2-p15/bin/bash

# This is the sample Linux notifier for V2 notifications.

# The notification payload is a JSON object presented to standard input,
# having the fields version, sender, message and mentioned. Note that
# the JSON data is always UTF-8 encoded. The script is responsible for
# decoding it to Unicode.

# This script depends upon `jq(1)` to extract fields from the JSON
# notification and upon `notify-send(1)` to invoke the system notifier.

# See docs/notification-scripts.md for further details.

PGM="`/nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/basename $0`"
function fail {
	echo $PGM: "$@" >/dev/stderr
	exit 1
}

# Confirm presence of `jq(1)` and `notify-send(1)`.'
function has {
	/nix/store/zw7zyp8nng1gqwbn1wc16yrwf3zg2zcc-which-2.21/bin/which $1 >/dev/null 2>&1
}
has jq || fail 'jq(1) is required but not present'
has notify-send || fail 'notify-send(1) is required but not present'

# Extract the data to be notified. The data is in a JSON payload at stdin.
json=`/nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/cat`
version=`echo $json|/nix/store/aj8lqifsyynq8iknivvxkrsqnblj7qzs-jq-1.6-bin/bin/jq -Mr .version`
[ "$version" = 2 ] || fail 'JSON payload has wrong version'
sender=`echo $json|/nix/store/aj8lqifsyynq8iknivvxkrsqnblj7qzs-jq-1.6-bin/bin/jq -Mr .from`
message=`echo $json|/nix/store/aj8lqifsyynq8iknivvxkrsqnblj7qzs-jq-1.6-bin/bin/jq -Mr .message`
mentioned=`echo $json|/nix/store/aj8lqifsyynq8iknivvxkrsqnblj7qzs-jq-1.6-bin/bin/jq -Mr .mention`

# Now prepare the notification. The logic is essentially the same as for
# the V1 notifier.

# Configure the notifier.
ns_URGENCY_GENERAL=normal    # none, low, normal or critical
ns_URGENCY_MENTIONED=normal  # none, low, normal or critical
ns_CATEGORY=im.received
ns_HEADER="Matterhorn message from @$sender"
ns_BODY="$message"

case "$mentioned" in
false) ns_URGENCY=$ns_URGENCY_GENERAL ;;
true) ns_URGENCY=$ns_URGENCY_MENTIONED ;;
*) fail "mentioned: invalid value: $mentioned" ;;
esac

# Emit the notification.
case "$ns_URGENCY" in
none) ;;
low|normal|critical)
	/nix/store/5xx01m9hw51zd6pm1vp7rpdkp9fng4by-libnotify-0.8.2/bin/notify-send -u $ns_URGENCY -c $ns_CATEGORY -- "$ns_HEADER" "$ns_BODY" ;;
*) fail "urgency not recognized: $ns_URGENCY" ;;
esac

# Log the JSON notification payload. To enable, define NOTIFY_JSON_LOG as
# a path to a file. The file need not exist; the directory must be writable.
NOTIFY_JSON_LOG=
[ -n "$NOTIFY_JSON_LOG" ] && {
	echo `/nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/date -Iminutes` $json >> $NOTIFY_JSON_LOG
}
true

### resholve directives (auto-generated) ## format_version: 3
# resholve: keep /nix/store/5xx01m9hw51zd6pm1vp7rpdkp9fng4by-libnotify-0.8.2/bin/notify-send
# resholve: keep /nix/store/aj8lqifsyynq8iknivvxkrsqnblj7qzs-jq-1.6-bin/bin/jq
# resholve: keep /nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/basename
# resholve: keep /nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/cat
# resholve: keep /nix/store/apn3p2b40xvirn7w740wv2gy330ppib5-coreutils-9.3/bin/date
# resholve: keep /nix/store/zw7zyp8nng1gqwbn1wc16yrwf3zg2zcc-which-2.21/bin/which
@Diamondy4 Diamondy4 changed the title Function argument substitution failed. Function argument substitution failed Sep 20, 2023
@abathur
Copy link
Owner

abathur commented Sep 20, 2023

Is this a bug?

Not really a bug, per se, but it is something resholve can't handle yet--so it's fine to have an open issue about it.

Currently using keep and replaceStrings as a fallback, but it's quite hacky.

If I were resolving this script, I'd probably just patch out the definition and invocations of this function (it's just there to sanity check something we should be able to ~guarantee, and cutting it also enables you to drop the which dependency).

I'd like to have a system where you can work around cases like this by telling resholve that the function has executable arguments, but then you also need the ability to tell resholve which arguments are executable. My first swing at the latter wasn't really usable. resholve uses argparse parsers to do the same work for fairly common commands, but there isn't yet a pluggable mechanism that would let you tack on your own.

I don't have time just now, but I'll try to follow up later with some notes on actually resolving these for posterity.

@abathur
Copy link
Owner

abathur commented Sep 21, 2023

Shell is too dynamic for a 100% solution to be feasible, so resholve generally aims to grow towards something like a 80-90%ish sort of solution.

This class of resolution basically requires resholve to see through a dynamic execution to understand that it in turn executes something else that would accept an executable as its argument. This is thematically in-scope; I've already taken steps towards doing that semi-reliably with well-known external executables that exec their arguments. Your specific example of this class is simple and only requires one level of resolution, but the broader class of problem has to wrestle with some thorny questions since all of shell's behavior is technically fair game.

Here are some example functions staking out a few quick edge cases:

run_all(){
  "$@"
}
run_one(){
  "$1"
}
run_with_options(){
  case "$1" in
    echo)
      shift
      echo "running command: $@"
      "$@"
      ;;
    log)
      shift
      "$@" > logfile
      ;;
    *)
      "$@"
      ;;
}

And then some invocations of those:

# both should get resolved here, since it'll run which, and we should resolve args to which
# even though they aren't executed
run_all which jq

# which should get resolved because it'll get executed
run_one which

# even though this looks like the same case as `run_all which jq`
# jq shouldn't get resolved this time--the function ~ignores its 2nd arg
run_one which jq 

# shouldn't resolve any; they'll get passed to the echo builtin
run_all echo which jq

# but should resolve which and jq, since the function will shift arg1 off
# before running the rest
run_with_options echo which jq

A robust solution to this will probably be out of reach for a while. Biting off the simplest cases may be tractable, but part of the trick there is being able to separate simple and complex cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants