Skip to content

Conversation

@rjmholt
Copy link
Collaborator

@rjmholt rjmholt commented Dec 12, 2019

PR Summary

I hit an issue trying to delete a directory under OneDrive like this:

At C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\PowerShellEditorServices.build.ps1:126 char:5
+     Remove-Item $PSScriptRoot\.tmp -Recurse -Force -ErrorAction Ignor …
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices [master ≡ +0 ~2 -0 !] [DBG]>> s
At C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\PowerShellEditorServices.build.ps1:127 char:5
+     Remove-Item $PSScriptRoot\module\PowerShellEditorServices\bin -Re …
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices [master ≡ +0 ~2 -0 !] [DBG]>> Remove-Item "$root/module/PowerShellEditorServices/bin" -Recurse -Force -Verbose
Remove-Item: Index (zero based) must be greater than or equal to zero and less than the size of the argument list.

Expanding the error revealed this:


ProviderInvocationException :
    ProviderInfo   : Microsoft.PowerShell.Core\FileSystem
    ErrorRecord    :
        Exception             :
            TargetSite :
                Name          : AppendFormatHelper
                DeclaringType : System.Text.StringBuilder
                MemberType    : Method
                Module        : System.Private.CoreLib.dll
            StackTrace :
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.Management.Automation.Internal.StringUtil.Format(String formatSpec, Object o)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveDirectoryInfoItem(DirectoryInfo directory, Boolean recurse, Boolean force, Boolean rootOfRemoval)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveItem(String path, Boolean recurse)
   at System.Management.Automation.SessionStateInternal.RemoveItem(CmdletProvider providerInstance, String path, Boolean recurse, CmdletProviderContext context)
            Message    : Index (zero based) must be greater than or equal to zero and less than the size of the argument list.
            Source     : System.Private.CoreLib
            HResult    : -2146233033
        CategoryInfo          : InvalidOperation: (:) [], FormatException
        FullyQualifiedErrorId : RemoveItemProviderException
    Message        : Attempting to perform the RemoveItem operation on the 'FileSystem' provider failed for path 'C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\module\PowerShellEditorServices\bin'. Index (zero based) must be greater than or equal to zero and less than the size of the argument
list.
    TargetSite     :
        Name          : RemoveItem
        DeclaringType : System.Management.Automation.SessionStateInternal
        MemberType    : Method
        Module        : System.Management.Automation.dll
    StackTrace     :
   at System.Management.Automation.SessionStateInternal.RemoveItem(CmdletProvider providerInstance, String path, Boolean recurse, CmdletProviderContext context)
   at System.Management.Automation.SessionStateInternal.RemoveItem(String providerId, String path, Boolean recurse, CmdletProviderContext context)
   at Microsoft.PowerShell.Commands.RemoveItemCommand.ProcessRecord()
   at System.Management.Automation.CommandProcessor.ProcessRecord()
    InnerException :
        TargetSite :
            Name          : AppendFormatHelper
            DeclaringType : System.Text.StringBuilder
            MemberType    : Method
            Module        : System.Private.CoreLib.dll
        StackTrace :
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.Management.Automation.Internal.StringUtil.Format(String formatSpec, Object o)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveDirectoryInfoItem(DirectoryInfo directory, Boolean recurse, Boolean force, Boolean rootOfRemoval)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveItem(String path, Boolean recurse)
   at System.Management.Automation.SessionStateInternal.RemoveItem(CmdletProvider providerInstance, String path, Boolean recurse, CmdletProviderContext context)
        Message    : Index (zero based) must be greater than or equal to zero and less than the size of the argument list.
        Source     : System.Private.CoreLib
        HResult    : -2146233033
    Source         : System.Management.Automation
    HResult        : -2146233087
ProviderInfo                : Microsoft.PowerShell.Core\FileSystem
ErrorRecord                 :
    Exception             :
        TargetSite :
            Name          : AppendFormatHelper
            DeclaringType : System.Text.StringBuilder
            MemberType    : Method
            Module        : System.Private.CoreLib.dll
        StackTrace :
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.Management.Automation.Internal.StringUtil.Format(String formatSpec, Object o)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveDirectoryInfoItem(DirectoryInfo directory, Boolean recurse, Boolean force, Boolean rootOfRemoval)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveItem(String path, Boolean recurse)
   at System.Management.Automation.SessionStateInternal.RemoveItem(CmdletProvider providerInstance, String path, Boolean recurse, CmdletProviderContext context)
        Message    : Index (zero based) must be greater than or equal to zero and less than the size of the argument list.
        Source     : System.Private.CoreLib
        HResult    : -2146233033
    CategoryInfo          : NotSpecified: (:) [Remove-Item], FormatException
    FullyQualifiedErrorId : System.FormatException,Microsoft.PowerShell.Commands.RemoveItemCommand
    InvocationInfo        :
        MyCommand        : Remove-Item
        ScriptLineNumber : 1
        OffsetInLine     : 1
        HistoryId        : 34
        Line             : Remove-Item "$root/module/PowerShellEditorServices/bin" -Recurse -Force -Verbose
        PositionMessage  : At line:1 char:1
                           + Remove-Item "$root/module/PowerShellEditorServices/bin" -Recurse -For …
                           + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        InvocationName   : Remove-Item
        CommandOrigin    : Internal
    ScriptStackTrace      : at <ScriptBlock>, C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\PowerShellEditorServices.build.ps1: line 127
                            at *Task, C:\Users\roholt\Documents\PowerShell\Modules\InvokeBuild\5.4.1\Invoke-Build.ps1: line 526
                            at <ScriptBlock><End>, C:\Users\roholt\Documents\PowerShell\Modules\InvokeBuild\5.4.1\Invoke-Build.ps1: line 684
                            at <ScriptBlock>, <No file>: line 1
    PipelineIterationInfo :



TargetSite                  :
    Name          : Invoke
    DeclaringType : System.Management.Automation.Runspaces.PipelineBase
    MemberType    : Method
    Module        : System.Management.Automation.dll
StackTrace                  :
   at System.Management.Automation.Runspaces.PipelineBase.Invoke(IEnumerable input)
   at System.Management.Automation.PowerShell.Worker.ConstructPipelineAndDoWork(Runspace rs, Boolean performSyncInvoke)
   at System.Management.Automation.PowerShell.CoreInvokeHelper[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings)
   at System.Management.Automation.PowerShell.CoreInvoke[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings)
   at System.Management.Automation.PowerShell.InvokeWithDebugger(IEnumerable`1 input, IList`1 output, PSInvocationSettings settings, Boolean invokeMustRun)
   at System.Management.Automation.ScriptDebugger.ProcessCommand(PSCommand command, PSDataCollection`1 output)
   at Microsoft.PowerShell.ConsoleHost.InputLoop.ProcessDebugCommand(String cmd, Exception& e)
Message                     : Index (zero based) must be greater than or equal to zero and less than the size of the argument list.
Data                        : System.Collections.ListDictionaryInternal
InnerException              :
    TargetSite :
        Name          : AppendFormatHelper
        DeclaringType : System.Text.StringBuilder
        MemberType    : Method
        Module        : System.Private.CoreLib.dll
    StackTrace :
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.Management.Automation.Internal.StringUtil.Format(String formatSpec, Object o)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveDirectoryInfoItem(DirectoryInfo directory, Boolean recurse, Boolean force, Boolean rootOfRemoval)
   at Microsoft.PowerShell.Commands.FileSystemProvider.RemoveItem(String path, Boolean recurse)
   at System.Management.Automation.SessionStateInternal.RemoveItem(CmdletProvider providerInstance, String path, Boolean recurse, CmdletProviderContext context)
    Message    : Index (zero based) must be greater than or equal to zero and less than the size of the argument list.
    Source     : System.Private.CoreLib
    HResult    : -2146233033
Source                      : System.Management.Automation
HResult                     : -2146233087

The directory looked like this:

 gi "$root/module/PowerShellEditorServices/bin"  | fl * -force

PSPath              : Microsoft.PowerShell.Core\FileSystem::C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\module\PowerShellEditorServices\bin
PSParentPath        : Microsoft.PowerShell.Core\FileSystem::C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\module\PowerShellEditorServices
PSChildName         : bin
PSDrive             : C
PSProvider          : Microsoft.PowerShell.Core\FileSystem
PSIsContainer       : True
Mode                : la---
ModeWithoutHardLink : la---
BaseName            : bin
Target              :
LinkType            :
Parent              : C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\module\PowerShellEditorServices
Root                : C:\
FullName            : C:\Users\roholt\OneDrive - Microsoft\Dev\PowerShellEditorServices\module\PowerShellEditorServices\bin
Extension           :
Name                : bin
Exists              : True
CreationTime        : 12/10/2019 3:41:06 PM
CreationTimeUtc     : 12/10/2019 11:41:06 PM
LastAccessTime      : 12/12/2019 9:49:09 AM
LastAccessTimeUtc   : 12/12/2019 5:49:09 PM
LastWriteTime       : 12/10/2019 3:41:06 PM
LastWriteTimeUtc    : 12/10/2019 11:41:06 PM
Attributes          : Directory, Archive, ReparsePoint

I've tried writing tests that look like this:

    Describe -Name "Symbolic links" -Tag "RequireAdminOnWindows" {
        BeforeEach {
            $testDir = New-Item -Path $TestDrive -Name "testdir" -ItemType Directory
        }

        AfterEach {
            if (Test-Path $testDir)
            {
                Remove-Item $testDir -Force -Recurse
            }
        }

        It "Should be able to delete a symlink to the directory" {
            $subDir = New-Item -ItemType Directory -Path $testDir -Name "subdir"
            New-Item -ItemType File -Path $subDir -Name "txt" -Value "nothing to see here"
            $symlink = New-Item -Name "kwyjibo" -Path $TestDrive -ItemType SymbolicLink -Target $testDir

            Remove-Item $symlink -Recurse -Force

            Test-Path $testDir | Should -BeTrue
            Get-ChildItem $testDir | Should -HaveCount 0
        }

        It "Should fail properly when a symlink cannot be deleted" {
            $subDir = New-Item -ItemType Directory -Path $testDir -Name "subdir"
            New-Item -ItemType File -Path $subDir -Name "txt" -Value "nothing to see here" -Force
            $file = New-Item -ItemType File -Path $subDir -Name "txt" -Value "nothing to see here"
            $symlink = New-Item -Name "kwyjibo" -Path $TestDrive -ItemType SymbolicLink -Target $testDir

            $file = [System.IO.File]::Open($file, 'open', 'read', 'none')
            try
            {
                Remove-Item $symlink -Recurse -Force

                Test-Path $testDir | Should -BeTrue
                Get-ChildItem $testDir | Should -HaveCount 0
            }
            finally
            {
                $file.Close()
                $file.Dispose()
            }
        }
    }

However, they don't fail like OneDrive symlinks do and I'm not sure how to emulate this behaviour.

This is a very simple fix though: the recurse parameter needed to be passed to Delete() and the error message needed its second argument passed through.

PR Checklist

@rjmholt rjmholt requested a review from anmenaga as a code owner December 12, 2019 19:04
@ghost ghost assigned anmenaga Dec 12, 2019
@rjmholt rjmholt added WG-Cmdlets-Management cmdlets in the Microsoft.PowerShell.Management module WG-Engine-Providers built-in PowerShell providers such as FileSystem, Certificates, Registry, etc. Issue-Bug Issue has been identified as a bug in the product labels Dec 12, 2019
@iSazonov
Copy link
Collaborator

I do not review in depth but I have concerns because there is old history:
#621
#3637
#3674

@anmenaga
Copy link

after looking at those linked items, I agree with @iSazonov . We need more info before we can safely merge this. Specifically:

  1. what happens before and after this change when target directory is not empty; this change is adding recurse=true argument to DirectoryInfo.Delete(Boolean) and looking at linked items that may end as a breaking change.
  2. what is so special about OneDrive symlinks
  3. maybe we can separate fixing the error message into a separate PR; as far as I understand that does not depend on DirectoryInfo.Delete(Boolean) modification?

@rjmholt
Copy link
Collaborator Author

rjmholt commented Jan 27, 2020

maybe we can separate fixing the error message into a separate PR; as far as I understand that does not depend on DirectoryInfo.Delete(Boolean) modification?

I'm happy with that.

what is so special about OneDrive symlinks

Well the answer to that seems to be "they require recurse set to delete". But I would like to know the full answer as well.

@rjmholt
Copy link
Collaborator Author

rjmholt commented Jan 27, 2020

But agreed. I think the bottom line is that if different symlinks behave differently, we need a way to differentiate them and get the right behaviour for each and this PR doesn't do that. I'll reduce it back to the error message fix.

Comment on lines +3052 to +3056
// TODO:
// Different symlinks seem to vary by behavior.
// In particular, OneDrive symlinks won't remove without recurse,
// but the .NET API here does not allow us to distinguish them.
// We may need to revisit using p/Invokes here to get the right behavior
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we remove the comment too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just put it in in the last commit, since the current behaviour won't work for OneDrive symlinks (why I opened the PR).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is better to open a tracking issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about both? Comments like this often help me when I find them in the code, because I usually hit them when I'm trying to fix something that I didn't realise was related.

@anmenaga anmenaga changed the title Fix symlink deletion for OneDrive Fix error message during symlink deletion Jan 30, 2020
@anmenaga anmenaga merged commit 8ad4997 into PowerShell:master Jan 30, 2020
@anmenaga anmenaga added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 30, 2020
@iSazonov iSazonov removed WG-Cmdlets-Management cmdlets in the Microsoft.PowerShell.Management module WG-Engine-Providers built-in PowerShell providers such as FileSystem, Certificates, Registry, etc. Issue-Bug Issue has been identified as a bug in the product labels Jan 31, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Jan 31, 2020
@ghost
Copy link

ghost commented Mar 26, 2020

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants