- Notifications
You must be signed in to change notification settings - Fork8.1k
Fix for Move-Item with wildcard source and a non-existent destination directory implodes first directory#26603
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
alexprabhat99 commentedDec 11, 2025
@daxian-dbw Could you please review this PR as potential fix for the issue:#24837? Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull request overview
This PR fixes a bug whereMove-Item with wildcard source paths to a non-existent destination directory would flatten the directory structure. The issue occurred because the first directory in the wildcard expansion was renamed to the destination path instead of being moved into it as a subdirectory, causing subsequent items to be moved into that first directory incorrectly.
Key changes:
- Adds logic to pre-create the destination directory when it doesn't exist but its parent does
- Adds a comprehensive test case to verify directory structure is preserved during wildcard moves
- Makes single directory move behavior consistent with wildcard move behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/System.Management.Automation/namespaces/FileSystemProvider.cs | ModifiedMoveItem method to create the destination directory before moving if it doesn't exist but the parent directory does, ensuring consistent move-into behavior |
test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 | Added test case that verifies directory structure preservation when moving wildcard contents to a non-existent destination directory |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| Copy-Item-Path$zeroLengthFile-Destination"$TestDrive\zeroLengthFile2.txt"-Force | ||
| "$TestDrive\zeroLengthFile2.txt"| Should-Exist | ||
| } | ||
CopilotAIDec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Consider adding a test comment explaining what specific bug this test is guarding against. While the test name is descriptive, a brief comment mentioning issue#24837 would help future maintainers understand the historical context and the specific flattening bug that was fixed.
| # Regression test for issue #24837: Ensure Move-Item with wildcards preserves directory structure and does not flatten contents. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We never add references to issues.
| Directory.CreateDirectory(destination); | ||
| } | ||
| } | ||
CopilotAIDec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Remove trailing whitespace at the end of this line to maintain code cleanliness and consistency with project standards.
| if(!ItemExists(destination)) | ||
| { | ||
| stringparentDir=Path.GetDirectoryName(destination); | ||
| if(!string.IsNullOrEmpty(parentDir)&&Directory.Exists(parentDir)) | ||
| { | ||
| Directory.CreateDirectory(destination); | ||
| } | ||
| } |
CopilotAIDec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This fix introduces a breaking change in behavior for single directory moves to non-existent destinations. Previously,Move-Item source/dir -Destination dest/newname (wheredest exists butnewname doesn't) would renamedir tonewname. With this fix, it createsnewname and then movesdir into it, resulting indest/newname/dir. This breaks the existing test "Verify Move-Item across devices for directory" at line 155 of FileSystem.Tests.ps1, which expects the directory contents to be at$destination/file.txt, not$destination/dir/file.txt. While this change makes wildcard and non-wildcard moves consistent, it's a breaking change that should be explicitly acknowledged and documented. The PR description incorrectly marks "User-facing changes: Not Applicable".
| if(!ItemExists(destination)) | |
| { | |
| stringparentDir=Path.GetDirectoryName(destination); | |
| if(!string.IsNullOrEmpty(parentDir)&&Directory.Exists(parentDir)) | |
| { | |
| Directory.CreateDirectory(destination); | |
| } | |
| } | |
| // Restore previous behavior: if destination does not exist, and its parent exists, | |
| // treat as a rename (do not create the destination directory). | |
| // Only create the destination directory if moving multiple items or using wildcards. | |
| // For single directory move, do not create destination directory. | |
| // (No action needed here; skip creating destination directory.) | |
| // If needed, logic for wildcard moves should be handled elsewhere. | |
| // (Intentionally left blank to restore previous behavior.) |
iSazonov commentedDec 11, 2025
@alexprabhat99 Please look how Copilot fixed the issue in#26604. Copilot started with fix like yours and did some iterations to avoid side effects. If you agree with that fix you can grab it. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Fixes:#24837
PR Context
This PR fixes the follwing scenario:
When you call Move-Item with a wildcard into a directory that doesn't exist yet, it seems like the first move of a directory acts as a directory creation, which ends up moving the files into the new directory, instead of creating a new sub-directory, and I don't understand why / find this a rather odd behavior.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header