Enable creating relative symlinks with New-Item#8783
Enable creating relative symlinks with New-Item#8783anmenaga merged 6 commits intoPowerShell:masterfrom
New-Item#8783Conversation
|
Codefactor issues are on untouched code and incorrectly identifying Hungarian notation |
|
|
|
Looking |
|
The spelling error in static analysis is unrelated to this PR. The CodeFactor issues are also unrelated to the changes in this PR. |
test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Management/New-Item.Tests.ps1
Outdated
Show resolved
Hide resolved
…em.Tests.ps1 Co-Authored-By: SteveL-MSFT <[email protected]>
|
@anmenaga I believe this is ready to merge |
|
I have only one open question above. |
src/System.Management.Automation/engine/SessionStateContainer.cs
Outdated
Show resolved
Hide resolved
iSazonov
left a comment
There was a problem hiding this comment.
LGTM with one minor comment.
vexx32
left a comment
There was a problem hiding this comment.
Nothing I can see that hasn't already been mentioned; looks good! 🙂
Co-Authored-By: SteveL-MSFT <[email protected]>
|
Thanks for fixing this.
I just tried the new code on my Mac, and it was able to create a symlink with a relative path just fine.
Similarly, I suggest enabling the tests for Unix-like platforms too. |
|
@mklement0 I tried it on my macBook and it didn't create a relative link. Not sure why it's different for me as the tests failed on my system and was also failing in CI. |
|
To verify that Describe "Relative symlink path test" {
It "ln creates a symlink with a relative path when given one" {
ln -s . /tmp/$pid
readlink /tmp/$pid | should -BeExactly '.'
rm /tmp/$pid
}
} |
|
@SteveL-MSFT, inferring the correct symlink type on Windows seems to be broken with relative paths to existing directories - see #9127 On a related note, we must come up with a way to explicitly specify the target type for nonexistent (not-yet-extant) targets - see #9067 (comment) |
PR Summary
The current code always tries to glob the target path which results in an absolute path and limits the usefulness of creating a symlink. Fix is to see if the target path even contains any wildcards, if not, don't use the resolved globbed path and instead use the value as literal. Since there isn't a
-LiteralTargetparameter, this does mean that the globber can fail if the target path contains a wildcard character intended to be literal, but that is not in scope of this PR.Note that relative links are only supported on Windows so skipping test on non-Windows. On non-Windows, the relative path is passed to the symlink() api which ends up creating an absolute link. Using the
lncommand line tool has the same result.PR Context
Fix #3500
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests