8000 Fix New-Item to create correct symlink type (#2915) by jeffbi · Pull Request #3509 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fix New-Item to create correct symlink type (#2915) #3509

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 14, 2017
Merged

Fix New-Item to create correct symlink type (#2915) #3509

merged 1 commit into from
Apr 14, 2017

Conversation

jeffbi
Copy link
Contributor
@jeffbi jeffbi commented Apr 7, 2017 •
edited by daxian-dbw
Loading

Fix #2915
Now creates a file symlink to a file target and to an non-existent target, and a directory symlink to a directory target.

Creating Links with New-Item

The New-Item cmdlet allows for creating file system links.
In general, New-Item can create four types of links:

  • Hard Link
    • Created when -ItemType is HardLink
  • Symbolic Link
    • Created when -ItemType is SymbolicLink and either the target exists and is a file, or the target does not exist
      • In Windows, if the target is a directory then New-Item -ItemType Symbolic Link will create a directory symbolic link, not a "plain" symbolic link.
      • In Linux and OS X, New-Item -ItemType Symbolic Link will always create a "plain" symbolic link, regardless of whether the target is a file or directory, or whether the target exists or not.
    • In Windows, elevated privileges are required to create a symbolic link.
  • Directory Symbolic Link
    • Created when -ItemType is SymbolicLink and the target exists and is a directory
    • Supported only on NTFS file system
    • In Windows, elevated privileges are required to create a directory symbolic link.
    • In Linux and OS X, New-Item will not create a directory symbolic link, even on an NTFS volume.
  • Directory Junction
    • Supported only on NTFS file system
    • In Linux and OS X, New-Item will not create a directory junction, even on an NTFS volume.
      • Currently, New-Item will begin to create the junction by creating the link directory. When the attempt to turn the newly-created directory into a junction fails, New-Item will delete the new directory and silently fail.

The following table shows the types of links and targets the New-Item cmdlet can create on Windows:

Link Type / TargetExisting FileExisting DirectoryNon-Existent"Plain" Symbolic LinkDirectory Symbolic LinkDirectory Junction
Hard Link Yes No No Yes No No
"Plain" Symbolic Link Yes No1 Yes Yes No1 No1
Directory Symbolic Link No1 Yes No1 No1 Yes Yes
Directory Junction No Yes No No Yes Yes
1 While the NTFS file system and the Windows operating system support "plain" symbolic links to existing directories and *directory* symbolic links to files and to non-existent items, the `New-Item` cmdlet cannot create them.

The following table shows the types of links and targets the New-Item cmdlet can create on Linux and OS X:

Link Type / TargetExisting FileExisting DirectoryNon-Existent"Plain" Symbolic LinkDirectory Symbolic LinkDirectory Junction
Hard Link Yes No No Yes No No
"Plain" Symbolic Link Yes Yes Yes Yes No No
Directory Symbolic Link2 No No No No No No
Directory Junction2 No No No No No No
2 The Linux and OS X operating systems do not support *directory* symbolic links or directory junctions.

Fix New-Item to create correct symlink type (#2915)
8085018 
Now creates a file symlink to a file target and to an non-existent target, and a directory symlink to a directory target.

Also updated tests.
iSazonov
iSazonov reviewed Apr 8, 2017
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1
$fileInfo.Target | Should Match ([regex]::Escape($FullyQualifiedFolder))
$fileInfo.LinkType | Should Be "SymbolicLink"
$fileInfo.Attributes -band $DirLinkMask | Should Be $DirLinkMask

# Remove the link explicitly to avoid broken symlink issue
Remove-Item $FullyQualifiedLink -Force
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, is what still relevant?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here Remove-Item is workaround for a bug. Is the bug already fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. I didn't know what "broken symlink issue" the author was referring to, so I didn't remove another dev's code. I am working on a Remove-Item issue, but I want to get this in so I can write tests using New-Item rather than shelling out a mklink command.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it's about problem with recursion when there are symlink. It is seems already fixed. It is not related with your PR.
Closed.

@iSazonov
Copy link
Collaborator
iSazonov commented Apr 8, 2017

LGTM.

@jeffbi
Copy link
Contributor Author
jeffbi commented Apr 10, 2017

@joeyaiello, @SteveL-MSFT Can whoever else needs to review this please do so? I have another issue that is waiting on this one to finish updating its PR, then I'll likely want to use changes from that one in yet another issue.

@joeyaiello joeyaiello mentioned this pull request Apr 10, 2017
Closed
@mirichmo mirichmo self-assigned this Apr 11, 2017
@SteveL-MSFT
Copy link
Member

cc @PowerShell/area-providers

@iSazonov
Copy link
Collaborator

@jeffbi Thanks very much for great PR description!

@jeffbi jeffbi mentioned this pull request Apr 14, 2017
Merged
@iSazonov
Copy link
Collaborator

@mirichmo @SteveL-MSFT The PR block #3441. 😕 Could you please continue with the PR?

@daxian-dbw
Copy link
Member
daxian-dbw commented Apr 14, 2017 •
edited
Loading

@jeffbi could you please help me to understand how the change in FileSystemProvider.cs fixed the issue?
It looks to me there is only one behavior change after the fix -- an exception will be thrown from CheckItemExists in windows, when NativeMethods.GetFileAttributes fails, and in that case, New-Item will return with an error message. It's not very obvious how this fixes the issue #2915. Could you please elaborate/explain the fix a bit?

@daxian-dbw
Copy link
Member

@jeffbi Never mind, isDirectory is the key here. The change looks good.

@daxian-dbw daxian-dbw merged commit 06020f3 into PowerShell:master Apr 14, 2017
@iSazonov
Copy link
Collaborator

@daxian-dbw Thanks for quick response!

@jeffbi jeffbi deleted the new-item-2915 branch April 24, 2017 23:22
@SteveL-MSFT SteveL-MSFT mentioned this pull request Apr 26, 2017
Closed
@mklement0 mklement0 mentioned this pull request Mar 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iSazonov iSazonov iSazonov left review comments

Assignees

@mirichmo mirichmo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeffbi @iSazonov @SteveL-MSFT @daxian-dbw @msftclas @mirichmo
0