cspotcode / npm-pwsh

Install PowerShell via npm for use in npm scripts or elsewhere.
12 stars 5 forks source link

Install Miscalculated pwsh.exe location on global install #9

Closed fearthecowboy closed 5 years ago

fearthecowboy commented 5 years ago

On Windows, using:

nvs to install node 8.12.0 npm install -g get-powershell`

C:\Users\garretts\AppData\Local\nvs\default\pwsh -> C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell\bin\pwsh

> get-powershell@0.1.0 postinstall C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell
> node ./npm_lifecycle_postinstall.js

Downloading Powershell archive from https://github.com/PowerShell/PowerShell/releases/download/v6.1.0/PowerShell-6.1.0-win-x64.zip to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64.zip...
Download finished.
Extracting archive to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64...
Extracted to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64
Creating .cmd shim from C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\pwsh.exe...
Done!
+ get-powershell@0.1.0
added 10 packages from 4 contributors in 57.763s

C:\Users\garretts>pwsh
The system cannot find the path specified.

Tracking down where it went:

C:\Users\garretts>which pwsh.cmd
C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd

C:\Users\garretts>more C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd
@"%~dp0\node_modules\get-powershell\bin\pwsh"   %*

C:\Users\garretts>more C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell\bin\pwsh.cmd
@"%~dp0\..\..\..\..\..\..\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\pwsh.exe"   %*

resolve-path C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell\bin\..\..\..\..\..\..\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\
resolve-path : Cannot find path 'C:\Users\garretts\AppData\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\' because it does
not exist.

Actual path should be two less ..\ -- ie ( @"%~dp0\..\..\..\..\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\pwsh.exe" %*

fearthecowboy commented 5 years ago

Works in a local install fine tho'

cspotcode commented 5 years ago

Thank you for reporting this.

What's confusing to me: why does the relative path include so many ..\ when it doesn't need that many? For example, there's no need to ..\..\default\node_modules because both the shim .cmd and target .exe are within default\node_modules.

Can you send me the output of npm config get prefix? And check if its capitalization is different from the canonical directory names?

fearthecowboy commented 5 years ago
> npm config get prefix
C:\Users\garretts\AppData\Local\nvs\default
fearthecowboy commented 5 years ago

Exactly the same capitalization.

cspotcode commented 5 years ago

Thank you, can I bug you to also checkout the "debug-9" branch and then run:

./scripts/build.ps1 -compile -package
npm install

I patched the shim creation code to log the path of the intermediate .cmd shim, just in case I made a mistake computing that path.

EDIT: it should output something like the following:

Creating .cmd shim from C:\Users\abradley\Documents\Personal-dev\@cspotcode\.bin\pwsh.cmd to C:\Users\abradley\.npm-get-powershell\powershell-6.1.0-win32-x64\pwsh.exe (via C:\Users\abradley\Documents\Personal-dev\@cspotcode\npm-get-powershell\bin\pwsh)...
fearthecowboy commented 5 years ago
#install locally built package
PS >npm install -g .\get-powershell-0.1.0.tgz

C:\Users\garretts\AppData\Local\nvs\default\pwsh -> C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell\bin\pwsh

> get-powershell@0.1.0 postinstall C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell
> node ./npm_lifecycle_postinstall.js

Downloading Powershell archive from https://github.com/PowerShell/PowerShell/releases/download/v6.1.0/PowerShell-6.1.0-win-x64.zip to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64.zip...
Download finished.
Extracting archive to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64...
Extracted to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64
Creating .cmd shim from C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd to C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\pwsh.exe (via C:\Users\garretts\AppData\Local\nvs\node\8.12.0\x64\node_modules\get-powershell\bin\pwsh)...
Done!
+ get-powershell@0.1.0
added 10 packages from 4 contributors in 47.772s

# try it
PS>pwsh
The system cannot find the path specified.

# nope! arg!
PS>which pwsh.cmd
C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd

# and that says!
PS>more C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd
@"%~dp0\node_modules\get-powershell\bin\pwsh"   %*

# which leads to ...
PS>more C:\Users\garretts\AppData\Local\nvs\default\node_modules\get-powershell\bin\pwsh
"$basedir/../../../../../../default/node_modules/@cspotcode/get-powershell-cache/powershell-6.1.0-win32-x64/pwsh.exe"   "$@"
exit $?

Still six ..\ ...

cspotcode commented 5 years ago

Yeah, I didn't actually attempt to fix the problem; I just wanted to diagnose it. But I found what I needed:

Creating .cmd shim from
C:\Users\garretts\AppData\Local\nvs\default\pwsh.cmd
to
C:\Users\garretts\AppData\Local\nvs\default\node_modules\@cspotcode\get-powershell-cache\powershell-6.1.0-win32-x64\pwsh.exe
(via
C:\Users\garretts\AppData\Local\nvs\node\8.12.0\x64\node_modules\get-powershell\bin\pwsh
)...

Looks like default is a symlink to node\8.12.0\x64. So I just need to account for that. Fix incoming...

fearthecowboy commented 5 years ago

given that you know where the target exe is and the cmd, why not just use :

    // Windows platforms: use cmd-shim
    if(process.platform === 'win32') {
        log(`Creating .cmd shim from ${ linkPath } to ${ targetPath } (via ${ intermediateLinkPath })...`);
        fs.writeFileSync(linkPath, `@"${targetPath}" %*` )
    }
fearthecowboy commented 5 years ago

I did that and it worked great (and skips the extra .cmd file redirection)

fearthecowboy commented 5 years ago

ah, yeah, nvs probably does some symlinking to manage different versions.

cspotcode commented 5 years ago

npm uninstall complains that the .cmd shim was not created by npm and can't be deleted. So I had to do the redirection to avoid modifying the .cmd that npm generates. :/

fearthecowboy commented 5 years ago

Hmm. I see that.

Still, the second cmd file? full path instead of relative path?

Thinking about what nvs/nvm will do, it might be a good idea to install the binary outside of the node_modules folder, even in the 'global' case.

fearthecowboy commented 5 years ago

FYI:

if(process.platform === 'win32') {
        log(`Creating .cmd shim from ${ linkPath } to ${ targetPath } (via ${ intermediateLinkPath })...`);
        fs.writeFileSync(`${intermediateLinkPath}.cmd`, `@"${targetPath}" %*` )
        fs.writeFileSync(`${intermediateLinkPath}`, `@"${targetPath}" %*` )
    }

worked just fine too.

cspotcode commented 5 years ago

Fixed in v0.1.1. Let me know if it works for you.

To answer your other questions:

I think we need to piggy-back on npm's bin mechanism since that's the one added to the user's $PATH and managed by npm on our behalf.

We're already using relative paths in the shims. npm config get prefix gives us a full path, but we use it to compute a relative path for the shim scripts.

Keeping all 3 executables (2x .cmd and .exe) within the same npm prefix directory ensures that the relative paths in the shims never have to ../ past a symlink. I'm calling fs.realpathSync to ensure all paths are resolved to non-symlinks.

The extra .cmd shouldn't incur a performance hit; they'll both execute within the same cmd process, right?

FYI The extension-less shim is meant for bash, so you can't use @ or %* syntax.

And FWIW "nvm" doesn't use symlinks; it modifies your $PATH. But either way, problem's fixed; should be good.