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

After upgrading to 7.1 No quote function is defined: https://ï.at/no-quote-func #524

Closed
geoffreysamper opened this issue Oct 6, 2022 · 15 comments

Comments

@geoffreysamper
Copy link

geoffreysamper commented Oct 6, 2022

Expected Behavior

when running a command below expect that the command it echo the hostname.

await $`echo ${process.env.HOSTNAME}`

Actual Behavior

Error: No quote function is defined: https://ï.at/no-quote-func
    at Proxy.quote (file:https:///usr/local/lib/node_modules/zx/build/core.js:38:15)
    at Proxy.set (file:https:///usr/local/lib/node_modules/zx/build/core.js:74:19)
    at file:https:///entrypoint.mjs:1:8
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:385:24)
    at async importPath (file:https:///usr/local/lib/node_modules/zx/build/cli.js:164:5)
    at async main (file:https:///usr/local/lib/node_modules/zx/build/cli.js:95:5)

If i go to the prev version of zx 7.0.8 everything works

Steps to Reproduce the Problem

Specifications

  • Version: zx 7.1.0
  • Platform: alpine
@cyntler
Copy link

cyntler commented Oct 6, 2022

@geoffreysamper I wonder what's the difference between your code:

await $`echo ${process.env.HOSTNAME}`

and the one in readme (https://github.com/google/zx#passing-env-variables):

await $`echo $FOO`

The effect is probably the same, but I'm just wondering because I haven't tested it.


What is the effect when you use it like below?

await $`echo $HOSTNAME`

@antonmedv
Copy link
Collaborator

Alpine does not have bash. Only sh (csh probably?) Zx doesn't have a quote function for csh, as zx uses $ escape bash feature.

You can fix it by adding bash:

RUN apk update && apk add bash

@geoffreysamper
Copy link
Author

when adding bash to it alpine image it works

@geoffreysamper
Copy link
Author

When running using the bash variable instead of the process it works

await $`echo $HOSTNAME`
output: b972da089044

@antonmedv
Copy link
Collaborator

When running using the bash variable instead of the process it works

Didn't get it.

@geoffreysamper
Copy link
Author

when running await $echo $FOO zx on run the command without error.

So changing the statement from

await $` echo ${process.env.FOO}` 

to

await $`echo $FOO`

@geoffreysamper
Copy link
Author

Is there a reason why bash is a requirement for zx. The puprose of the ZX is to use javascript and bash statement together. Isn't the quote function the same for most linux shells?

@antonmedv
Copy link
Collaborator

Unfortunately no. Bash has this nice quote C strings: https://www.gnu.org/software/bash/manual/html_node/ANSI_002dC-Quoting.html

@antonmedv
Copy link
Collaborator

@maitrungduc1410
Copy link

Alpine does not have bash. Only sh (csh probably?) Zx doesn't have a quote function for csh, as zx uses $ escape bash feature.

You can fix it by adding bash:

RUN apk update && apk add bash

you saved me. This definitely needs to be specified in README

@svenjacobs
Copy link

I have a similar problem. I currently try to write a Raycast script with zx. Unfortunately Raycast launches scripts in a non-login /bin/sh shell, so I get the same error (No quote function is defined). I currently struggle in finding the right shebang to launch the script in Bash and execute zx.

#!/bin/bash -l -c zx

doesn't work as it just executes the zx command without passing the scripts contents. Any idea?

@atsu85
Copy link

atsu85 commented Apr 21, 2023

Probably the safest option is still to install bash to avoid issues when quoting is actually needed, but i thought i'll share my findings as well.

I managed to get it working on alpine with following patch for ash shell:

diff --git a/node_modules/zx/build/core.js b/node_modules/zx/build/core.js
index 6de412b..3354f76 100644
--- a/node_modules/zx/build/core.js
+++ b/node_modules/zx/build/core.js
@@ -49,6 +49,17 @@ catch (err) {
     if (process.platform == 'win32') {
         defaults.shell = which.sync('powershell.exe');
         defaults.quote = quotePowerShell;
+    } else {
+        if (process.platform == 'linux') {
+            try {
+                defaults.shell = which.sync('ash');
+                defaults.prefix = 'set -euo pipefail;';
+                defaults.quote = quote;
+            }
+            catch (err) {
+                // ignore - unknown linux shell
+            }
+        }
     }
 }
 function getStore() {

but i haven't tested it it also works if quoting would actually be needed (my use-case doesn't actually need quoting).

To make it backwards compatible with v7.0.8, better solution would be to modify default quote implementation, so it wouldn't fail if quoting is needed. @antonmedv, what do you think?

@antonmedv
Copy link
Collaborator

Let's stick with the bash.

@andrewmclagan
Copy link

Supporting sh and alpine would be pretty great for docker builds

@antonmedv
Copy link
Collaborator

sh in alpine uses variant of https://linux.die.net/man/1/ash which does not support bash $`...` syntax. And zx relies on this escape mechanism.

But you can install bash in the docker container in a separate layer. It should be straightforward.

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

7 participants