-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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.
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
win_copy: Add missing check_mode fix #31469
win_copy: Add missing check_mode fix #31469
Conversation
@@ -76,7 +76,9 @@ Function Copy-File($source, $dest) { | |||
} | |||
|
|||
if (Test-Path -Path $dest -PathType Leaf) { | |||
Remove-Item -Path $dest -Force -Recurse | Out-Null | |||
if (-not $check_mode) { | |||
Remove-Item -Path $dest -Force -Recurse | Out-Null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would simply do:
Remove-Item -Path $dest -Force -Recurse -WhatIf:$check_mode | Out-Null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, got miscarried by earlier comments that avoided WhatIf because the path didn't exist; which is not the case here as the if conditional is Test-Path.... (facepalm)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. In reality there won't be a real difference, on the other hand if Microsoft at some point improves Remove-Item to also detect other failure-cases (in a safe way) the -WhatIf
implementation may prove to be more useful.
If Shippable likes it, I will merge this. Thanks ! |
* Add missing check_mode fix * Refactor conditional to use the one embedded in Remove-Item (cherry picked from commit 5bf9f27)
Thanks for the fix @vtsingaras, I have cherry-picked this commit to stable-2.4 https://github.com/ansible/ansible/commits/stable-2.4 for the 2.4.1 release. |
* Add missing check_mode fix * Refactor conditional to use the one embedded in Remove-Item
SUMMARY
win_copy would remove the destination file if it existed even when running with check_mode: yes
ISSUE TYPE
COMPONENT NAME
win_copy
ANSIBLE VERSION
2.4.0