shx icon indicating copy to clipboard operation
shx copied to clipboard

Support cross platform environment variables

Open backspaces opened this issue 7 years ago • 18 comments

Add a cross-env feature which resolves environment variables in a standard way.

Example: I like to use env vars so that configuration information can be stored in package.json. I believe the following will fail on windows but works fine on mac:

  "mkdirs": "dist docs/dist docs/models",
  "scripts": {
    "clean": "shx rm -rf $npm_package_mkdirs && shx mkdir $npm_package_mkdirs",

backspaces avatar Feb 06 '18 20:02 backspaces

BTW: This works, but is pretty ugly:

"clean": "cross-env-shell \"shx rm -rf $npm_package_mkdirs && shx mkdir $npm_package_mkdirs\"",

.. but I don't have a windows environment to check this works there. It may simply work on mac/linux anyway.

backspaces avatar Feb 06 '18 21:02 backspaces

Just to clarify, npm_package_mkdirs is an environmental variable?


#65 mentions the cross-env package too.

This sounds like a reasonable feature for shx (shelljs already exposes this under shell.env.NAME_OF_VAR). In general, shell variables can be quite complex due to string manipulation (I implemented a subset of bash-isms at nfischer/shelljs-transpiler#5).

If we want to support this, we need to decide how far we should go:

  • Basic support: $FOO ➡️ shell.env.FOO
  • Brackets: ${FOO} ➡️ shell.env.FOO
  • Prefix/suffix: prefix${FOO}suffix ➡️ 'prefix' + shell.env.FOO + 'suffix'
  • And so many more edge cases...

@backspaces what do you think fits the needs for most users?

nfischer avatar Feb 07 '18 00:02 nfischer

Yes, the code above is part of my package.json which I used to store configuration information for scripts. Looks like this:

  "mkdirs": "dist docs/dist docs/models",
  "scripts": {
    "clean": "cross-env-shell \"shx rm -rf $npm_package_mkdirs && shx mkdir $npm_package_mkdirs\"",
    "build": "npm-run-all clean build-libs build-dist minify build-docs",
    "build-libs": "node bin/wraplibs.js",
    "build-dist": "rollup -c",
    "build-docs": "shx cp -R README.md dist docs && node bin/docsmodels.js",
    "minify": "squash dist/AS.js > dist/AS.min.js && squash dist/AS.module.js > dist/AS.module.min.js",
    "test": "ava --verbose"
  },

So in my use-case, I would like

shx rm -rf $npm_package_mkdirs

To be expanded to

shx rm -rf dist docs/dist docs/models

I think that would be the basic support case.

I'm not sure if you'd need a way to set env vars. I haven't run across that.

backspaces avatar Feb 07 '18 01:02 backspaces

Oh, I see. I wasn't familiar that npm package attributes were set as environmental variables. Thanks for explaining.

Another thing we have to worry about: $npm_package_mkdirs could contain spaces, and POSIX shells would expand that into multiple words. We're a bit constrained here. We have no way to properly distinguish when the user passes quotes:

shx rm -rf $npm_package_mkdirs
# This is an 'rm' with 3 directory arguments

shx rm -rf "$npm_package_mkdirs"
# This has 1 directory argument (contains spaces in the name)

Would it be reasonable to support only single-word package variables?

nfischer avatar Feb 07 '18 05:02 nfischer

Would it be reasonable to support only single-word package variables?

I'm not sure what you mean by that. If it means my 3 dir example fails, then nope! :)

But seriously, I believe the usual expansion of $foo is simply the value, spaces and all, of the $foo env var. Your second examples is fine for when the spaces are really part of the file name.

As for spaces in the dir names, couldn't the programmer simply quote them like this: export foo='1 "2 3"' which produces: echo $foo => 1 "2 3"

Note that env vars can come from lots of different files, not just package.json. AND they needn't be env vars but simply vars like so in npm scripts:

   "foo": "files='one \"two three\"' && shx echo $files"

This is likely not cross-platform, however. But wouldn't shx export files='one \"two three\"' be cool!

Maybe the issue is node support. I found this: https://www.twilio.com/blog/2017/08/working-with-environment-variables-in-node-js.html .. mainly to emphasize their popularity.

backspaces avatar Feb 07 '18 17:02 backspaces

To simplify:

  • I would look into how node handles env vars: https://nodejs.org/dist/latest-v8.x/docs/api/all.html#process_process_env
  • And have shx replace $foo with the string process.env returns, within the shx command string.

backspaces avatar Feb 07 '18 18:02 backspaces

As for spaces in the dir names, couldn't the programmer simply quote them like this: export foo='1 "2 3"' which produces: echo $foo => 1 "2 3"

Try this:

$ export foo='first_file.txt "second file with spaces.txt"'
$ for file in $foo; do echo $file; done
first_file.txt
"second
file
with
spaces.txt"
$ # Yikes! I was hoping for 1 file per line

Side note: this is why bash has arrays. But you can't put arrays in env vars, so that isn't useful for shx.

Note that env vars can come from lots of different files, not just package.json. AND they needn't be env vars but simply vars like so in npm scripts:

Non-env vars are infeasible: these are scoped to the current shell, so shx (which runs as a subprocess) can't look-up their value.

Maybe the issue is node support

The issue is that npm runs scripts in a shell (/bin/sh or cmd.exe). Shells manipulate CLI arguments before launching the sub process (in our case, that's shx). Imagine we've set a variable like so:

$ export foo='bar     baz'

Consider how this looks to the shx process:

shx command desired output (unix) process.argv.slice(3) on unix process.argv.slice(3) on Windows
shx echo $foo bar baz [ 'bar', 'baz'] ['$foo']
shx echo "$foo" bar·····baz [ 'bar·····baz'] ['$foo']
shx echo '$foo' $foo [ '$foo'] I forget exactly, either ['$foo'] or ["'$foo'"]

(Pretend that the · characters are spaces, markdown rendering squashes consecutive whitespace)

Our first big problem is that every case looks the same on Windows. If we can't distinguish, then which behavior do we choose? If we assume any of the cases, then we've broken the others.

nfischer avatar Feb 08 '18 03:02 nfischer

Sigh. Brilliant explanation, thank you.

My current solution uses cross-env-shell, in the second post above. I wonder how they handle it? We could check their code.

But I believe their problem is different: they mainly convert between $foo and %foo% formats and they let the unix/windows shells handle it.

Historically: I got here by initially using bash backticks to create arguments for commands:

"foo": "rm -rf `pkgkey.js('mkdirs')`

where pkgkey simply reads in package.json, converts it to a JS object and returns the key's value as a string. It's a hack, but see below.

However, I soon found that backticks don't work in windows. So finally discovered env vars as a solution. I don't suppose backticks make sense for shx?

Here's pkgkey.js:

#!/usr/bin/env node
const fs = require('fs')
const json = JSON.parse(fs.readFileSync('package.json'))

const key = process.argv[2]
if (!key) throw new Error('pkgkey called with no argument')
let val = json[key]
if (!val) throw new Error(`pkgkey: ${key} not found`)

if (Array.isArray(val)) val = val.join(' ')

process.stdout.write(val)

backspaces avatar Feb 08 '18 18:02 backspaces

Hmm. Just a thought: would backticks be easier to implement than env vars?

backspaces avatar Feb 08 '18 18:02 backspaces

My current solution uses cross-env-shell, in the second post above. I wonder how they handle it? We could check their code.

They're accepting a full command string. So process.argv looks more like ['shx cp $files "$dest"'] instead of ['cp', '$files', '$dest']. The tradeoff is that they need to do all the parsing that a shell would normally do, but they get to see exactly what the user typed.

However, I soon found that backticks don't work in windows. So finally discovered env vars as a solution. I don't suppose backticks make sense for shx?

Not exactly our purpose. We're trying to expose shelljs command implementations to the CLI, not execute arbitrary commands.


If you're not opposed to writing a script, how about using shelljs directly?

// package.json
{
...
  "mkdirs": [ "first dir", "second dir", "third dir" ],
...
}
// clean.js (hook this up to `npm run clean`)
var shell = require('shelljs');

var dirList = require('./package.json').mkdirs;

var ret = shell.rm('-rf', ...dirList);
if (ret.code === 0) shell.mkdir(...dirList);

nfischer avatar Feb 08 '18 21:02 nfischer

Nice approach! I may do that.

I use shelljs in nearly all my node utilities. Like the es6 usage, thanks. And didn't know I could require a json file, that is cool.

backspaces avatar Feb 08 '18 23:02 backspaces

Yeah, unfortunately it seems like there's no obvious way to support env variables without running into these edge cases. It's a good feature request, and it would've been really nice if we had a path forward.

For tough cases (and this appears to be one) we still need to resort back to ShellJS. While a short script isn't as elegant as a shell one-liner, it offers more flexibility and power.

nfischer avatar Feb 09 '18 07:02 nfischer

@nfischer I would guess upwards of 90% of use-cases are super simple single word variables, e.g. $OPTION that would resolve to true or false or some path without spaces.

Can we not start with this very simple support and work our way up to more complex edge cases as we go? The limitations can be documented and the edge-cases can be addressed incrementally.

I think this is a great tool and it certainly made my life easier, but, for me, this one small thing would make a lot of difference.

itaysabato avatar Feb 28 '18 16:02 itaysabato

Can we not start with this very simple support and work our way up to more complex edge cases as we go?

The issue is that supporting that case would break other cases. Basically, cp '$foo.txt' would behave differently than shx cp '$foo.txt', which may surprise users.

The limitations can be documented and the edge-cases can be addressed incrementally.

Yes, we can document our behavior. But just to reiterate, we cannot address edge cases incrementally: we face fundamental limitations which are impossible to resolve.

If we were to implement this, the best case we could achieve is:

  • only support exported environmental variables
  • always assume $foo means "expand $foo to its value"
    • breaks cases like shx echo 'Print literal $foo' (could be controlled by a switch)
  • expand $foo as if it were double-quoted (expand it to one string, preserving white space)
    • doesn't satisfy the original use case for this bug report (could be controlled by a switch)
  • too difficult to support all the bashisms for manipulating strings (e.g. ${foo}, ${#foo}, ${foo/a/b}, etc.)
    • possible to implement a subset, but not reasonable to get everything
  • can't distinguish cases like "$foo"bar and "$foobar"

Basically, we have to assume what the user intends, which means sometimes we'll assume incorrectly. We could provide switches to assume the opposite way, but I'm not promising these would be good ideas or reasonable to support.

nfischer avatar Feb 28 '18 23:02 nfischer

@nfischer: btw, the example you show of for file in $foo; do echo $file; done is the bane of bash programmers. I.e. spaces in any name is a nightmare. Even for bash.

How do bash programmers solve it? A common solution is read

http://wiki.bash-hackers.org/commands/builtin/read

I know I typically do that:

echo 'a b c' | while read file; do echo $file; done

Produces:

a b c

For files, consider this directory, bar with two files "a" and "b c"

ls -1 a b c

And

ls -1 | while read file; do echo $file; done

.. handles file names with spaces.

I think shx's ls does fine: shx ls a b c

So basically, I'd say env vars with spaces are the user's problem, there are MANY way to handle them, just don't use spaces for both separators and in file names. So export foo="a:b c" would use : for separator and spaces in file names.

Here's how to say what $foo does in the shx/shelljs world: exactly the same as process.env.foo does. Period. If I need to deal with edge cases, that's my concern. Just like with bash! : )

backspaces avatar Mar 02 '18 22:03 backspaces

Can we just start with node_env? Pretty much every single user of this package will be using node_env sometime in their scripts, and it's a pain that on OSX and Linux it's 'node_env="production"' and on Windows it's 'set node_env="production"'. cross-env solves this problem, but I would like a simple solution in shx. Another option is if there are if statements that can run in shx, and you can do one for windows and the other for everything else.

frastlin avatar Dec 01 '19 19:12 frastlin

What about setting the env vars in the same line, without having to restort to cross-env? Eg:

"scripts": {
    "build:dev": "shx NODE_ENV=development webpack"
    "build": "shx NODE_ENV=production webpack",
    ....

goerwin avatar Feb 03 '21 04:02 goerwin

@goerwin we don't support executing arbitrary commands at the moment, so I don't think there's any benefit to defining variables in-line (although I can see the benefit of combining these features together). Other than that, this is subject to the same concerns I outlined above (distinguishing between $foo vs. '$foo', VAR=value vs. 'VAR=value', etc.).

nfischer avatar Feb 05 '21 20:02 nfischer