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

Postinstall not fit for Windows #181

Open
waiblinger opened this issue Apr 21, 2022 · 23 comments · May be fixed by Omrisnyk/npm-lockfiles#146
Open

Postinstall not fit for Windows #181

waiblinger opened this issue Apr 21, 2022 · 23 comments · May be fixed by Omrisnyk/npm-lockfiles#146
Assignees
Labels
question Further information is requested

Comments

@waiblinger
Copy link

waiblinger commented Apr 21, 2022

Hi,
since the update yesterday I can't install my modules, since the postinstall script uses "||", which isn`t supported on Windows in the given context.

`At line:1 char:53
+  node -e "try{require('./_postinstall')}catch(e){}" || exit 0
+                                                     ~~
The token '||' is not a valid statement separator in this version.
    + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : InvalidEndOfLine`
@medikoo
Copy link
Owner

medikoo commented Apr 21, 2022

@waiblinger thanks for the report, what terminal are you using on Windows? I was actually testing it on Windows prior release and all worked well.

@medikoo medikoo added the question Further information is requested label Apr 21, 2022
@waiblinger
Copy link
Author

I use the integrated terminal in Visual Studio Code (so cmd). The project ran yesterday befor the changes.

@waiblinger
Copy link
Author

@medikoo I tried powershell as well. Same problem

@ilyachalov
Copy link

@medikoo I'm trying to install 'gulp-cli' from 'npm' and I have the same problem (Windows 10):

windows10-powershell

|| is pipeline chain operator. They appeared in 'PowerShell 7':

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_pipeline_chain_operators

'Windows Powershell' (version 5.1) comes with 'Windows 10', executable file 'powershell.exe'.
'Powershell 7' can be obtained separately, executable file 'pwsh.exe'.
'Windows Powershell' (version 5.1) and 'Powershell 7' installed side by side.

'Windows Powershell' (version 5.1) does not have an pipeline chain operator ||.

I think the line
powershell -c node -e "try{require('./_postinstall')}catch(e){}" || exit 0
need to replace with
pwsh -c node -e "try{require('./_postinstall')}catch(e){}" || exit 0

Then it will work for those who have installed 'PowerShell 7'.

@medikoo
Copy link
Owner

medikoo commented Jul 4, 2022

I think the line
powershell -c node -e "try{require('./_postinstall')}catch(e){}" || exit 0
need to replace with
pwsh -c node -e "try{require('./_postinstall')}catch(e){}" || exit 0

It's actually an npm that adds powershell -c, so it'll be good to confirm if that happens with latest npm version, and eventually report issue over there.

@ilyachalov
Copy link

@medikoo Thanks for the reply. I've already figured it out.

npm can be configured in command line:

> npm config set script-shell="pwsh"

And now I have succeeded (I also installed 'PowerShell 7'):

windows10-pwsh-ok

Thanks again. 🎉🎉🎉

@mrJli
Copy link

mrJli commented Oct 13, 2022

Windows 11, PowerShell 7.2.6, i try this npm config set script-shell="pwsh" but
изображение

@ilyachalov
Copy link

ilyachalov commented Oct 13, 2022

@mrJli Are you really running the installation in PowerShell? PowerShell command prompt usually looks like this PS C:\>. You have the command prompt shown as $. Perhaps you are using 'Git Bash' (or some other shell) rather than 'PowerShell'?

Can you show a screenshot without cropping the window title?

@mrJli
Copy link

mrJli commented Oct 14, 2022

@mrJli Are you really running the installation in PowerShell? PowerShell command prompt usually looks like this PS C:\>. You have the command prompt shown as $. Perhaps you are using 'Git Bash' (or some other shell) rather than 'PowerShell'?

Can you show a screenshot without cropping the window title?

изображение

@ilyachalov
Copy link

@mrJli Earlier it was about a mistake with operator ||. To fix this error I used 'PowerShell 7' and 'npm' setting npm config set script-shell="pwsh".

You have a different situation. You have a unary operator --, which is not needed there:

pwsh -c --  node -e "try{require('./_postinstall')}catch(e){}" || exit 0

'es5-ext' is responsible for part of the command:
https://github.com/medikoo/es5-ext/blob/main/package.json

 node -e "try{require('./_postinstall')}catch(e){}" || exit 0

I think 'npm' is responsible for the initial part:

pwsh -c -- 

Maybe it's a bug in npm:
npm/cli#5332

What version of 'npm' do you have (npm --version, I have 8.12.1)? Maybe you should use a different version of 'npm'.

@mrJli
Copy link

mrJli commented Oct 15, 2022

npm version - 8.19.2
i try to install 8.12.1 and all works, thanks

@rjwut-chg
Copy link

I have zero issue with the message and intent behind the postinstall script, but perhaps unintended consequences like this are a good argument for limiting that sort of thing to the README.md file instead.

@kieselai
Copy link

This was quite frustrating to figure out, since I was calling my script from PowerShell core, but this postinstall script was still being called from PowerShell 5. I haven't had issues on any other machine I've ran it on, but this one apparently didn't have pwsh configured as the "script-shell".
Results on Google simply suggested using the compatible syntax for PowerShell 5, which isn't helpful when it's not a script I'm executing directly.
It wasn't until going directly to this repo and looking at the open issues that I saw what the solution was. It's obvious in hindsight, but was a bit of a runaround getting here.
I know cross compatibility with windows is a pain, but this is the first package I've ran into this particular issue.

If it helps, here are the versions of software/libs I'm running:
PowerShell: 7.3.4
Node: v18.15.0
pnpm: 8.6.3
npm: 9.4.0

And the solution for me was to set pwsh as the script-shell:
npm config set script-shell="pwsh"

@kieselai
Copy link

The more I look into it, I think the specific problem might occur if PowerShell 5 (or earlier) was previously specified as the script-shell. cmd.exe actually does allow "||" and "&&" operators, so I think the problem is specifically with older versions of PowerShell being the default script-shell.

@kieselai
Copy link

It would have been PowerShell 6 and earlier with the problem, my mistake

@medikoo
Copy link
Owner

medikoo commented Feb 23, 2024

Fixed with v0.10.63 release

@medikoo medikoo closed this as completed Feb 23, 2024
@mar777121
Copy link

Unfortunately it breaks gulp-cli installation, when using older node version:
eg. 16.17.0

@medikoo
Copy link
Owner

medikoo commented Feb 26, 2024

@mar777121 ensure you rely on latest v0.10.63, as mentioned above that issue was fixed

@mar777121
Copy link

Thanks @medikoo, I've checked, it does use the latest version.
Sorry, forgot to mention that the issue happens under alpine php latest image, using nvm as the package manager.

18.84 npm WARN deprecated source-map-url@0.4.1: See https://github.com/lydell/source-map-url#deprecated
18.84 npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
18.95 npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
19.06 npm WARN deprecated source-map-resolve@0.5.3: See https://github.com/lydell/source-map-resolve#deprecated
21.07 npm notice
21.07 npm notice New major version of npm available! 8.15.0 -> 10.4.0
21.07 npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.4.0
21.07 npm notice Run npm install -g npm@10.4.0 to update!
21.07 npm notice
21.07 npm ERR! code 127
21.07 npm ERR! path /root/.nvm/versions/node/v16.17.0/lib/node_modules/gulp-cli/node_modules/es5-ext
21.08 npm ERR! command failed
21.08 npm ERR! command sh /tmp/postinstall-cac6c5c8.sh
21.08 npm ERR! /tmp/postinstall-cac6c5c8.sh: line 1: node: Permission denied
21.08
21.08 npm ERR! A complete log of this run can be found in:
21.08 npm ERR! /root/.npm/_logs/2024-02-26T17_11_57_086Z-debug-0.log

@medikoo
Copy link
Owner

medikoo commented Feb 26, 2024

@mar777121 I understand that issue is, that this wasn't fixed completely, right? And not that patch introduced a new issue.
It's specific to Windows PowerShell (?)

@mar777121
Copy link

mar777121 commented Feb 26, 2024

@medikoo Indeed, it seems that while patch fixed the windows PowerShell issue, introduced a new issue, specific to linux.
I haven't tested on ubuntu, but at least on alpine I'm getting the error provided in my previous message.
Issue is not present while using node version >= v18

@medikoo
Copy link
Owner

medikoo commented Feb 26, 2024

@mar777121 indeed so apparently we got #143 back (I totally forgot about this case).

Is anyone aware of a way which may fix the issue for both of the cases?

@medikoo
Copy link
Owner

medikoo commented Feb 27, 2024

@mar777121 I've decided to revert that fix, it's published with v0.10.64.

If anyone knows to how to fix the Windows issue, without introducing issues for Linux systems I'll be grateful for a hint

@medikoo medikoo reopened this Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants