Skip to content

Fix path handling bug in PSTask - #12554

Merged
Aditya Patwardhan (adityapatwardhan) merged 5 commits into
PowerShell:masterfrom
IISResetMe:fix/ForEach-Object-Parallel-path-test
May 14, 2020
Merged

Fix path handling bug in PSTask#12554
Aditya Patwardhan (adityapatwardhan) merged 5 commits into
PowerShell:masterfrom
IISResetMe:fix/ForEach-Object-Parallel-path-test

Conversation

@IISResetMe

@IISResetMe Mathias R. Jessen (IISResetMe) commented May 2, 2020

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

@IISResetMe
Add ForEach-Object -Parallel path test for wildcard $PWD
1596c5c 
`ForEach-Object -Parallel` should work just the same when $PWD has wildcard characters in their names
@IISResetMe
Fix PowerShell#12428 incorrect literal path handling
323cceb
vexx32
Rain Sallow (/u/ta11ow) (vexx32) approved these changes May 2, 2020
View reviewed changes

@vexx32 Rain Sallow (/u/ta11ow) (vexx32) left a comment

Copy link
Copy Markdown
Collaborator

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. 😁

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 Outdated
@iSazonov

Ilya (iSazonov) commented May 3, 2020 •
edited by unfurl-links Bot
Loading

Copy link
Copy Markdown
Collaborator

Mathias R. Jessen (@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.

@IISResetMe @vexx32
Update test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-O…
7300d82 
…bject-Parallel.Tests.ps1

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

Aditya Patwardhan (adityapatwardhan) commented May 4, 2020

Copy link
Copy Markdown
Member

Mathias R. Jessen (@IISResetMe) Please fix the CodeFactor issues.

@iSazonov

Ilya (iSazonov) commented May 5, 2020

Copy link
Copy Markdown
Collaborator

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

Mathias R. Jessen (IISResetMe) commented May 7, 2020

Copy link
Copy Markdown
Collaborator Author

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

@iSazonov

Ilya (iSazonov) commented May 7, 2020

Copy link
Copy Markdown
Collaborator

Mathias R. Jessen (@IISResetMe) Do you want to work on general fix?

@IISResetMe

Mathias R. Jessen (IISResetMe) commented May 10, 2020

Copy link
Copy Markdown
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
Aditya Patwardhan (adityapatwardhan) merged commit 3c07aad into PowerShell:master May 14, 2020
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented May 14, 2020

Copy link
Copy Markdown
Member

Mathias R. Jessen (@IISResetMe) Thanks for your contribution!

@adityapatwardhan Aditya Patwardhan (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

Deleted user (ghost) commented Jun 25, 2020

Copy link
Copy Markdown

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

Handy links:

@ghost ghost mentioned this pull request Jun 25, 2020
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@IISResetMe
Fix path handling bug in PSTask (PowerShell#12554)
edd92c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
+1 more reviewer
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) vexx32 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ForEach-Object -Parallel incorrect path handling

4 participants

@IISResetMe @iSazonov @adityapatwardhan @vexx32