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

Fix path handling bug in PSTask #12554

Conversation

IISResetMe
Copy link
Collaborator

PR Summary

This PR changes the behavior PSTask to always treat the current $PWD as a literal path, in turn allowing ForEach-Object -Parallel to execute correctly when invoked from a path with wildcard characters in the name.

PR Context

Fixes #12428

PR Checklist

`ForEach-Object -Parallel` should work just the same when $PWD has wildcard characters in their names
@ghost ghost assigned adityapatwardhan May 2, 2020
Copy link
Collaborator

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, nice work!

Tests look good I think, just noticed a small style thing in one section that's really bugging me, but nothing serious. 😁

@iSazonov
Copy link
Collaborator

iSazonov commented May 3, 2020

@IISResetMe I have started a code review and discover that the fix is only partial. I found at least two scenario where wildcards broke set location:

  1. Create [] dir, change dir to the directory, then run cd -; cd +
  2. Create [] dir, change dir to the directory, then run pwsh - current directory will be pwsh home, not [].

Result of my investigations is in https://github.com/iSazonov/PowerShell/tree/fix-cd-literalpath
I was very grateful to you if you took this and supplemented with tests (maybe in new PR).

GitHub
PowerShell for every system! Contribute to iSazonov/PowerShell development by creating an account on GitHub.

…bject-Parallel.Tests.ps1

Co-authored-by: Joel Sallow (/u/ta11ow) <32407840+vexx32@users.noreply.github.com>
@adityapatwardhan
Copy link
Member

@IISResetMe Please fix the CodeFactor issues.

@iSazonov
Copy link
Collaborator

iSazonov commented May 5, 2020

I think it makes no sense to merge the PR because follow more general fix will revert it - the general fix is in another place.

@IISResetMe
Copy link
Collaborator Author

@iSazonov I... really don't care 😅 as long as #12428 gets fixed

@iSazonov
Copy link
Collaborator

iSazonov commented May 7, 2020

@IISResetMe Do you want to work on general fix?

@IISResetMe
Copy link
Collaborator Author

Yeah I can do a broader patch for this and related behavior, but might take a few days, looks like LiteralPath support has been neglected all over the place 😒

@adityapatwardhan adityapatwardhan merged commit 3c07aad into PowerShell:master May 14, 2020
@adityapatwardhan
Copy link
Member

@IISResetMe Thanks for your contribution!

@adityapatwardhan adityapatwardhan added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label May 14, 2020
@ghost
Copy link

ghost commented Jun 25, 2020

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Engine Indicates that a PR should be marked as an engine change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ForEach-Object -Parallel incorrect path handling
4 participants