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

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

Closed
anthony-c-martin opened this issue Jun 17, 2019 · 9 comments

Comments

@anthony-c-martin
Copy link

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
Copy link

Just hit the same problem today.

@cspotcode
Copy link
Owner

cspotcode commented Jul 19, 2019

The culprit is either:

A) Our createSymlinkTo function:

await promisify(cmdShim)(targetPath, intermediateLinkPath.replace(/\.cmd$/, ''));

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
Copy link
Owner

npm/cmd-shim#21

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

@cspotcode
Copy link
Owner

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
Copy link

Josverl commented Aug 13, 2019

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
Copy link

Josverl commented Aug 13, 2019

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
Copy link

Josverl commented Aug 15, 2019

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.

  • direct via invoke-pester or seperate pwsh invoke-pester works most often
  • , ./test/invoke-pester.ps1 also kindda works , but eats a lot more time
  • ./scripts/build.ps1 -test ive never seen working at all

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"
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.
  1. replace cmd.shim with "@zkochan/cmd-shim": "^3.1.0"
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; but this is not working , and for now this is what I can do.

@cspotcode
Copy link
Owner

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
Copy link
Owner

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

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

No branches or pull requests

4 participants