Fix FileSystemProvider to work with volume and pipe paths #15873

SteveL-MSFT · 2021-08-04T23:05:46Z

PR Summary

A change in the FileSystemProvider to get the correct casing for a path regressed working with NTFS volume paths that start with \\?\ as it's not a UNC path so skips that handling. Also, the logic for handling a trailing directory separator relied on there to be no content so something like: "c:\foo\\bar" would handle this incorrectly.

Fix is to skip the first parts of the path even if it's not a UNC path, but starts with "\\?\". Then we only check for empty content if we are at the end of the path to handle trailing separator correctly.

Also supports \\.\pipe on Windows

PR Context

Fix #15872

PR Checklist

src/System.Management.Automation/namespaces/FileSystemProvider.cs

fMichaleczek · 2021-08-05T10:38:06Z

@SteveL-MSFT Maybe good to take the #11898 in consideration while resolving this path problem.

Both examples work on Windows PowerShell

Get-ChildItem '//./pipe/'
Get-ChildItem '\\.\pipe\'

Found in this Rancher Wins documentation
https://github.com/rancher/wins#examples

GitHub
GitHub - rancher/wins: Windows containers connect to Windows host
Windows containers connect to Windows host. Contribute to rancher/wins development by creating an account on GitHub.

iSazonov · 2021-08-05T12:55:51Z

I'd merge #14469 first. Then we could start moving to new .Net API with #12834 and continue with other scenarios. After all, it would have corrected most of the issues we know in FileSystem provider.

src/System.Management.Automation/engine/Utils.cs

iSazonov · 2021-08-17T05:03:18Z

For reference #10805

ghost · 2021-08-24T14:00:38Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

adityapatwardhan · 2022-05-06T18:22:10Z

@SteveL-MSFT Can you resolve conflicts?

ghost · 2022-05-14T02:00:43Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

pull-request-quantifier-deprecated · 2022-06-29T05:05:38Z

This PR has 29 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Small
Size       : +19 -10
Percentile : 11.6%

Total files changed: 3

Change summary by file extension:
.cs : +11 -10
.ps1 : +8 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

SteveL-MSFT · 2022-07-25T21:16:19Z

@adityapatwardhan fixed merge conflict

test/powershell/Modules/Microsoft.PowerShell.Management/Get-ChildItem.Tests.ps1

…ildItem.Tests.ps1

ghost · 2022-08-12T00:00:47Z

🎉v7.3.0-preview.7 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

SteveL-MSFT requested a review from anmenaga as a code owner August 4, 2021 23:05

ghost assigned adityapatwardhan Aug 4, 2021

SteveL-MSFT requested a review from iSazonov August 4, 2021 23:05

iSazonov reviewed Aug 5, 2021

View reviewed changes

src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated Show resolved Hide resolved

iSazonov reviewed Aug 5, 2021

View reviewed changes

src/System.Management.Automation/engine/Utils.cs Outdated Show resolved Hide resolved

iSazonov reviewed Aug 6, 2021

View reviewed changes

src/System.Management.Automation/engine/Utils.cs Outdated Show resolved Hide resolved

SteveL-MSFT force-pushed the dir-iso branch from 223588a to f102b96 Compare August 13, 2021 21:00

ghost added the Review - Needed The PR is being reviewed label Aug 24, 2021

SteveL-MSFT force-pushed the dir-iso branch from 13ae508 to d755a76 Compare September 14, 2021 21:35

ghost removed the Review - Needed The PR is being reviewed label May 6, 2022

ghost added the Review - Needed The PR is being reviewed label May 14, 2022

SteveL-MSFT force-pushed the dir-iso branch from d755a76 to d2bb928 Compare June 20, 2022 20:37

SteveL-MSFT changed the title ~~Fix FileSystemProvider to work with volume paths~~ Fix FileSystemProvider to work with volume and pipe paths Jun 20, 2022

pull-request-quantifier-deprecated bot added the Extra Small label Jun 20, 2022

SteveL-MSFT force-pushed the dir-iso branch from d2bb928 to 150870f Compare June 29, 2022 05:05

SteveL-MSFT added 5 commits July 25, 2022 14:15

Fix FileSystemProvider to work with volume paths

e6044a5

address Ilya's feedback moving the check into PathIsUnc()

2dc83d2

add parameter to PathIsUnc to only check for Unc path

5e8c051

add support for \\.\pipe\

250dc65

update comment

e9963e4

SteveL-MSFT force-pushed the dir-iso branch from 150870f to e9963e4 Compare July 25, 2022 21:15

ghost removed the Review - Needed The PR is being reviewed label Jul 25, 2022

adityapatwardhan requested changes Jul 25, 2022

View reviewed changes

test/powershell/Modules/Microsoft.PowerShell.Management/Get-ChildItem.Tests.ps1 Outdated Show resolved Hide resolved

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 25, 2022

Update test/powershell/Modules/Microsoft.PowerShell.Management/Get-Ch…

ae2ad68

…ildItem.Tests.ps1

adityapatwardhan enabled auto-merge (squash) July 25, 2022 21:34

adityapatwardhan approved these changes Jul 25, 2022

View reviewed changes

adityapatwardhan merged commit d72cedd into PowerShell:master Jul 25, 2022

SteveL-MSFT deleted the dir-iso branch July 25, 2022 22:18

iSazonov added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jul 26, 2022

ghost mentioned this pull request Aug 12, 2022

Get-ChildItem of (Get-Volume) Paths does not work as expected #15872

Closed

5 tasks

mkht mentioned this pull request Aug 16, 2022

Windows (NTFS): Inconsistent support for the long-path prefix (\\?\) in filesystem paths #10805

Open

TravisEz13 mentioned this pull request Sep 30, 2022

Move ApiScan to compliance build #18191

Merged

22 tasks

Tadas mentioned this pull request Nov 2, 2022

Filesystem Provider, UNC Paths & Double Backslash #13633

Closed

Fix FileSystemProvider to work with volume and pipe paths #15873

Fix FileSystemProvider to work with volume and pipe paths #15873

Uh oh!

Conversation

SteveL-MSFT commented Aug 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Uh oh!

fMichaleczek commented Aug 5, 2021 • edited by unfurl-links bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Aug 5, 2021

Uh oh!

Uh oh!

Uh oh!

iSazonov commented Aug 17, 2021

Uh oh!

ghost commented Aug 24, 2021

Uh oh!

adityapatwardhan commented May 6, 2022

Uh oh!

ghost commented May 14, 2022

Uh oh!

pull-request-quantifier-deprecated bot commented Jun 29, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

SteveL-MSFT commented Jul 25, 2022

Uh oh!

Uh oh!

ghost commented Aug 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SteveL-MSFT commented Aug 4, 2021 •

edited

Loading

fMichaleczek commented Aug 5, 2021 •

edited by unfurl-links bot

Loading