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

[FEATURE] drop env's -S flag #54

Closed
yeldiRium opened this issue Jul 27, 2021 · 3 comments · Fixed by #96
Closed

[FEATURE] drop env's -S flag #54

yeldiRium opened this issue Jul 27, 2021 · 3 comments · Fixed by #96

Comments

@yeldiRium
Copy link
Contributor

What / Why

When writing shims for node.js application entry-points it happens that we want to pass additional flags to node when starting. env supports this via the -S flag. For example, one of my projects has this shebang in its entry-point:

#!/usr/bin/env -S node --loader ts-node/esm --experimental-specifier-resolution=node --no-warnings

The flag -S tells env to split the following text on whitespace and treat the segments as args to the executable in the first segment after the -S (i.e. node), instead of interpreting the rest of the line as one long command.

This works without issue on *nix based systems (tested on manjaro, ubuntu, macOS).

When

When using this module to create the shim for a package with an entry-point whose shebang uses the -S flag, the -S flag is interpreted as the name of the executable to run. This makes the resulting shim unusable.

Where

This is the absolute opposite of a minimal example, but the issue occurs in an ongoing PR here[1].
If needed I can build a small reproduction repository.

How

Current Behavior

The -S flag in the shebang is interpreted as the name of the executable that the shim should run.

Expected Behavior

I expect the -S flag to be ignored, since skipping it and using only the remaining shebang works on windows (according to my experiments).

Who

  • n/a

References

@nathanlepori
Copy link

Just leaving a comment for visibility. Any chance this is going to be merged soon?

@datenreisender
Copy link

I just tripped over this too, thanks for describing it here! Until this is merged which workarounds do you use?

I currently consider writing some additional scripts which then run the actual scripts. That is a pain but the best I currently imagine.

datenreisender added a commit to NordicSemiconductor/pc-nrfconnect-shared that referenced this issue May 2, 2022
Work around an issue on Windows with running scripts using `env -S`
npm/cmd-shim#54
@edemaine
Copy link

edemaine commented Aug 10, 2022

I'd argue that this is in fact a bug. Currently it's impossible to make "bin" point to Node code that needs command-line arguments and works cross-platform.

If one writes #!/usr/bin/env node --enable-source-maps (for example), then this works on platforms using shims: the shim runs node --enable-source-maps. However, on Linux where there are no shims, this fails because env receives a single argument node --enable-source-maps; env needs the -S flag to split this up.

If one writes #!/usr/bin/env -S node --enable-source-maps (for example), then this works on Linux. But on platforms using shims, the shim attempts to run -S node --enable-source-maps which of course fails.

A simple fix would be to use shims on Linux; then we wouldn't need -S at all.

But a probably better fix would be to detect -S when detecting /usr/bin/env in shim creation, and strip that off as well. Oh, #55 does this already.

The best workaround I found is to write a NodeJS wrapper that runs Node with the desired command-line options (so two Nodes end up running). Slows things down a bit, but at least it's functional.

edemaine added a commit to edemaine/svgtiler that referenced this issue Aug 10, 2022
Linux needs `#!/usr/bin/env -S` but this breaks Windows shims
<npm/cmd-shim#54>

Wrap in another layer of Node as a workaround.
@nlf nlf closed this as completed in #96 Dec 13, 2022
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

Successfully merging a pull request may close this issue.

4 participants