Removing Hardlink from Mode property in default file system format by powercode · Pull Request #8789 · PowerShell/PowerShell

powercode · 2019-01-30T00:33:13Z

Adding a new table view, childrenWithHardlink, that has the same behavior as children view had before this PR.

The children view now uses the modified ModeWithoutHardlink property that doesn't check for hardlinks when setting the l letter of Mode column.

The Mode property keeps it's existing behavior, so this change should not be breaking.

PR Summary

#8051

Significant speedup for default filesystem output

$files = gci -Recurse c:\windows -ea:0
$files | ft | out-null
$files | ft -view childrenWithHardlink  | out-null

view	time	speedup
childrenWithHardlink	3m22s	1x
children	7.6s	26x

Formatting result

Output before

PS> gci

    Directory: D:\repos\PowerShell\symlinks


Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d-----       2019-02-03     15:37                dir
d----l       2019-02-03     15:38                symdir
d----l       2019-02-03     15:39                symsubdir
-a----       2019-02-03     15:38              3 file
-a---l       2019-02-03     15:37              3 hardlink
-a---l       2019-02-03     15:38              0 symfile1

Output after

PS> gci

    Directory: D:\repos\PowerShell\symlinks

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-03    15:37                dir
l----         2019-02-03    15:38                symdir -> D:\repos\PowerShell\symlinks\dir
l----         2019-02-03    15:39                symsubdir -> D:\repos\PowerShell\symlinks\dir\subdir\
-a---         2019-02-03    15:38              3 file
-a---         2019-02-03    15:37              3 hardlink
la---         2019-02-03    15:38              0 symfile1 -> D:\repos\PowerShell\file

PS> gci | ft -view childrenWithHardlink

    Directory: D:\repos\PowerShell\symlinks

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-03    15:37                dir
l----         2019-02-03    15:38                symdir -> D:\repos\PowerShell\symlinks\dir
l----         2019-02-03    15:39                symsubdir -> D:\repos\PowerShell\symlinks\dir\subdir\
-a---         2019-02-03    15:38              3 file
la---         2019-02-03    15:37              3 hardlink
la---         2019-02-03    15:38              0 symfile1 -> D:\repos\PowerShell\file

produces the old output.

PR Context

PR Checklist

adityapatwardhan · 2019-01-30T17:39:42Z

@powercode The change looks good in general. Could you also put a before and after of how the output of Get-ChildItem looks like for future reference in the PR description.

adityapatwardhan · 2019-01-30T17:51:42Z

@powercode Could also have a look at the CodeFactor issue, some on them are about documentation and formating. Would be nice to get those fixed.

JamesWTruher · 2019-01-30T23:11:47Z

symlinks will still appear in the default view, right?
j

powercode · 2019-01-30T23:36:32Z

@JamesWTruher Yes, symlinks appear as an l.

powercode · 2019-01-31T08:24:49Z

The build errors seem unrelated to my change. Is there a way to trigger a build without a new commit?

adityapatwardhan · 2019-01-31T17:53:26Z

@powercode Restarted the CI.

adityapatwardhan · 2019-01-31T22:57:21Z

@powercode Please respond to the committee recommendation about re-using the d field for links instead of adding one at the end.

powercode · 2019-01-31T23:25:28Z

@adityapatwardhan I'm on it :)

Is this what we want, or should I skip the Link resolution?

    Directory: D:\repos\PowerShell\symlinktest

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-01    00:22                dir
l----         2019-02-01    00:23                symdir -> D:\repos\PowerShell\symlinktest\dir\subdir\
-a---         2019-02-01    00:20              3 file

    Directory: D:\repos\PowerShell\symlinktest\dir

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-01    00:22                subdir
-a---         2019-02-01    00:22              3 subfile
la---         2019-02-01    00:21              0 symfile1 -> D:\repos\PowerShell\symlinktest\file

SteveL-MSFT · 2019-01-31T23:45:34Z

@powercode yes, that's perfect!

powercode · 2019-01-31T23:48:15Z

@SteveL-MSFT I'm playing now with moving AddScriptBlockColumn into hidden code properties. Seems to be a bit faster to.

Any reason not to do that? The Name string needed it to get to the internal methods for Link management.

powercode · 2019-02-01T00:34:25Z

A bit of feature creep here :) Feel free to pick the commits you want!

adityapatwardhan · 2019-02-01T01:45:40Z

@powercode Please have a look at the test failure

SteveL-MSFT

We should have some tests validating the formating changes

powercode · 2019-02-01T23:50:58Z

I consider the remaining CodeFactor issues to be noise, and the CI static analysis seem to complain about spelling in MD files.

iSazonov

@SteveL-MSFT Has PowerShell Committee approved new public APIs?

powercode · 2019-02-03T15:54:49Z

This is what the default view and the childrenWithHardlink view looks like:

PS> gci
    Directory: D:\repos\PowerShell\symlinks

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-03    15:37                dir
l----         2019-02-03    15:38                symdir -> D:\repos\PowerShell\symlinks\dir
l----         2019-02-03    15:39                symsubdir -> D:\repos\PowerShell\symlinks\dir\subdir\
-a---         2019-02-03    15:38              3 file
-a---         2019-02-03    15:37              3 hardlink
la---         2019-02-03    15:38              0 symfile1 -> D:\repos\PowerShell\file

PS> gci | ft -view childrenWithHardlink

    Directory: D:\repos\PowerShell\symlinks

Mode                LastWriteTime         Length Name
----                -------------         ------ ----
d----         2019-02-03    15:37                dir
l----         2019-02-03    15:38                symdir -> D:\repos\PowerShell\symlinks\dir
l----         2019-02-03    15:39                symsubdir -> D:\repos\PowerShell\symlinks\dir\subdir\
-a---         2019-02-03    15:38              3 file
la---         2019-02-03    15:37              3 hardlink
la---         2019-02-03    15:38              0 symfile1 -> D:\repos\PowerShell\file

adityapatwardhan · 2019-03-12T01:04:21Z

@powercode I have pushed changes to convert the new xUnit tests to Pester. I also rebased to master to resolve some merge conflicts. Please have a look if I have missed something. And thank you for your patience.

iSazonov · 2019-03-12T06:53:07Z

-            }
+            return Platform.IsWindows
+                ? fileInfo.Attributes.HasFlag(System.IO.FileAttributes.ReparsePoint)
+                : Platform.NonWindowsIsSymLink(fileInfo);


I wonder why we use Platform.IsWindows not #if UNIX?

The only code path that used multiple return values was !CORECLR. I removed the obsolete #ifdef-ed code.

powercode · 2019-03-12T07:10:41Z

I adressed @daxian-dbw's feedback.
The other changes LGTM.

adityapatwardhan · 2019-03-12T17:40:38Z

@daxian-dbw Can you update your review?

daxian-dbw

LGTM

powercode · 2019-03-12T22:24:42Z

@daxian-dbw You won the race!
With several seconds :)

adityapatwardhan · 2019-03-12T22:50:22Z

@powercode Thanks for your contribution and patience!

lukeb1961 · 2020-01-12T23:04:40Z

How will a normal person using Powershell, know that these FT views exist?

iSazonov · 2020-01-13T03:07:30Z

@lukeb1961 Please open a feature request (for tab completion) and/or doc isuue.

powercode requested review from BrucePay, anmenaga and daxian-dbw as code owners January 30, 2019 00:33

powercode force-pushed the Perf/Hardlink branch from 62e83be to badf4c4 Compare January 30, 2019 00:40

SteveL-MSFT requested changes Jan 30, 2019

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/DefaultFormatters/FileSystem_format_ps1xml.cs Outdated

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

SteveL-MSFT assigned adityapatwardhan Jan 30, 2019

SteveL-MSFT added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jan 30, 2019

adityapatwardhan requested changes Jan 30, 2019

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/DefaultFormatters/FileSystem_format_ps1xml.cs Outdated

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jan 30, 2019

powercode force-pushed the Perf/Hardlink branch from 88b6d93 to 35a425f Compare February 1, 2019 00:27

powercode force-pushed the Perf/Hardlink branch from 35a425f to 9cbd8d8 Compare February 1, 2019 00:36

SteveL-MSFT requested changes Feb 1, 2019

View reviewed changes

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

SteveL-MSFT requested changes Feb 2, 2019

View reviewed changes

Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated

iSazonov added the Documentation Needed in this repo Documentation is needed in this repo label Feb 2, 2019

iSazonov reviewed Feb 2, 2019

View reviewed changes

adityapatwardhan force-pushed the Perf/Hardlink branch from 63bd965 to 15b1144 Compare March 12, 2019 01:02

adityapatwardhan approved these changes Mar 12, 2019

View reviewed changes

daxian-dbw requested changes Mar 12, 2019

View reviewed changes

iSazonov reviewed Mar 12, 2019

View reviewed changes

powercode added 2 commits March 12, 2019 08:06

isHardLink -> isLink

ad839f9

Changing signature of GetTarget from IEnumerable<string> to string.

6542c7f

The only code path that used multiple return values was !CORECLR. I removed the obsolete #ifdef-ed code.

Make InternalGetTarget function unix only and remove unreachable code

77fed92

Minor updates

5a699e4

daxian-dbw reviewed Mar 12, 2019

View reviewed changes

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

daxian-dbw approved these changes Mar 12, 2019

View reviewed changes

Add parenthesis to fix the precedence order

d568eda

adityapatwardhan merged commit 9983297 into PowerShell:master Mar 12, 2019

TravisEz13 modified the milestones: 6.2.0-GA-Consider, 6.2.0 Mar 21, 2019

SteveL-MSFT mentioned this pull request Sep 30, 2019

Is hardlinks really that important that they need to be in the Mode for fileinfo? #8051

Closed

SteveL-MSFT mentioned this pull request Oct 30, 2019

Get-ChildItem doesn't output correctly when used with Invoke-Command #10759

Open

iSazonov mentioned this pull request Jan 12, 2020

Powershell Core 6.2.3 does not show Hardlink Target as in previous versions. #11562

Closed

rjmholt mentioned this pull request Jan 18, 2020

Public APIs missing in PowerShell 7.0.0-rc.2 since 6.2.3 #11619

Closed

lukeb1961 mentioned this pull request Jan 20, 2020

Feature Request #8789 has new FT views, but no way for normal user to know about them. #11633

Closed

iSazonov mentioned this pull request Oct 19, 2022

Powershell v7.2.0-preview.4 file Hard Link .Target get nothing #15139

Open

Conversation

powercode commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Formatting result

Output before

Output after

PR Context

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adityapatwardhan commented Jan 30, 2019

Uh oh!

adityapatwardhan commented Jan 30, 2019

Uh oh!

JamesWTruher commented Jan 30, 2019

Uh oh!

powercode commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

powercode commented Jan 31, 2019

Uh oh!

adityapatwardhan commented Jan 31, 2019

Uh oh!

adityapatwardhan commented Jan 31, 2019

Uh oh!

powercode commented Jan 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SteveL-MSFT commented Jan 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

powercode commented Jan 31, 2019

Uh oh!

powercode commented Feb 1, 2019

Uh oh!

adityapatwardhan commented Feb 1, 2019

Uh oh!

SteveL-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

powercode commented Feb 1, 2019

Uh oh!

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

powercode commented Feb 3, 2019

Uh oh!

adityapatwardhan commented Mar 12, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iSazonov Mar 12, 2019

Choose a reason for hiding this comment

Uh oh!

powercode commented Mar 12, 2019

Uh oh!

adityapatwardhan commented Mar 12, 2019

Uh oh!

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

powercode commented Jan 30, 2019 •

edited

Loading

powercode commented Jan 30, 2019 •

edited

Loading

powercode commented Jan 31, 2019 •

edited

Loading

SteveL-MSFT commented Jan 31, 2019 •

edited

Loading