cspotcode / npm-pwsh

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

Pwsh.exe path linking fails if package directory is on a different disk to home directory #18

Closed anthony-c-martin closed 4 years ago

anthony-c-martin commented 5 years ago

On install, .\node_modules\pwsh\bin\pwsh and .\node_modules\pwsh\bin\pwsh.cmd contain a link to pwsh.exe in the user's home directory.

If the package.json is on a different disk drive to the user's home directory, this link is malformed - it assumes the path to the home directory is a relative path. For example, when running npm install with a package.json file on my E:, I see the following:

.\node_modules\pwsh\bin\pwsh.cmd:

@"%~dp0\C:\Users\antmarti\.npm-pwsh\powershell-6.1.0-win32-x64\pwsh.exe"   %*

.\node_modules\pwsh\bin\pwsh:

"$basedir/C:/Users/antmarti/.npm-pwsh/powershell-6.1.0-win32-x64/pwsh.exe"   "$@"
exit $?
weshaggard commented 5 years ago

Just hit the same problem today.

cspotcode commented 5 years ago

The culprit is either:

A) Our createSymlinkTo function: https://github.com/cspotcode/npm-pwsh/blob/0f7440d151e9331864b534b0a2b0717116619039/src/util.ts#L266

B) The third-party cmd-shim dependency which actually creates the .cmd file https://www.npmjs.com/package/cmd-shim

Can you try to narrow this down and see if it's a bug in cmd-shim? You should be able to create a minimal reproduction where you use cmd-shim's API to create a shim from one drive to another. See if the output is broken in the same way.

cspotcode commented 5 years ago

https://github.com/npm/cmd-shim/issues/21

Looks like this is, indeed, a cmd-shim bug.

cspotcode commented 5 years ago

It appears that cmd-shim is not receiving updates, but there is a fork that's used by pnpm and yarn.

https://www.npmjs.com/package/@zkochan/cmd-shim Used by yarn: https://github.com/yarnpkg/yarn/blob/master/package.json#L9

If anyone wants to fix this (@weshaggard or @antmarti-microsoft?) I recommend submitting a PR that switches to using @zkochan/cmd-shim. See if that fixes the problem. If not, we can submit a PR to https://github.com/pnpm/cmd-shim.

I unfortunately don't have the bandwidth to fix this now, but I'll merge a publish a PR.

Josverl commented 5 years ago

If/when switching implementation, is there any reason to prefer @zkochan/cmd-shim over npm/cmd-shim ?

according to the network diagram npm/cmd-shim seems to be the fork most recently/activly maintained

Josverl commented 5 years ago

I tried a quick look, but thus far I can't build or test based on my initial look at the instructions.

there seems to be a circular reference and I keep hitting issues in test testoutput.txt

quick attempt at some patches

Im willing to give a hand , but would need to have a stable baseline to start from

Josverl commented 5 years ago

OK , I gave up on getting the tests to provide any predictive or reliable results. ive tried to adjust / rewrite them , and while I understand pester quite well , i don't understand what you are trying to test. Ive tried to reverse engineer this , but this is taking way too much time. They always fail the first run , Sometimes they fail a subsequent second run , and other thimes they succeed if you run then a few times.

However on my machine is seems to give no indication at all weather or not the solution will actually work. I reverted to testing in real life by adding the tgz to a test project and deploying via azure pipelines.

  1. Update cmd-shim to "cmd-shim": "^3.0.0"
    • CI Logs pwsh 0.2.1
    • fails on windows , but gives not error ( False Positive)
      
      Extracting archive to C:\Users\VssAdministrator\.npm-pwsh\powershell-6.1.0-win32-x64...
      Extracted to C:\Users\VssAdministrator\.npm-pwsh\powershell-6.1.0-win32-x64
      Creating .cmd shim from D:\_work\1\s\node_modules\.bin\pwsh.cmd to C:\Users\VssAdministrator\.npm-pwsh\powershell-6.1.0-win32-x64\pwsh.exe (via D:\_work\1\s\node_modules\pwsh\bin\pwsh)...
      Done!

justatest@1.0.0 postinstall D:_work\1\s pwsh hello2.ps1

'find_dp0' is not recognized as an internal or external command, operable program or batch file. The filename, directory name, or volume label syntax is incorrect.


2. replace cmd.shim with "@zkochan/cmd-shim": "^3.1.0" 
  - [CI Logs pwsh 0.2.2](https://dev.azure.com/josverl/JosVerlinde/_build/results?buildId=65)
 - fails on windows : 

Extracting archive to C:\Users\VssAdministrator.npm-pwsh\powershell-6.1.0-win32-x64... Extracted to C:\Users\VssAdministrator.npm-pwsh\powershell-6.1.0-win32-x64 Creating .cmd shim from D:_work\1\s\node_modules.bin\pwsh.cmd to C:\Users\VssAdministrator.npm-pwsh\powershell-6.1.0-win32-x64\pwsh.exe (via D:_work\1\s\node_modules\pwsh\bin\pwsh)...

justatest@1.0.0 postinstall D:_work\1\s pwsh hello2.ps1

The filename, directory name, or volume label syntax is incorrect. npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! justatest@1.0.0 postinstall: pwsh hello2.ps1 npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the justatest@1.0.0 postinstall script.


happy to [share what I've tried](https://github.com/Josverl/npm-pwsh/tree/fix/symlink); but this is not working , and for now this is what I can do.
cspotcode commented 4 years ago

The tests have been updated to run on Github Actions in mocha, not pester. Pester was more trouble than it was worth.

If anyone wants to take another stab at fixing this bug, it should be easier now.

cspotcode commented 4 years ago

Replaced by #24 to more concisely describe the necessary fix to newcomers.