search

RamblingCookieMonster / PSDeploy

Simple PowerShell based deployments
http://ramblingcookiemonster.github.io/PSDeploy-Take-Two/
MIT License
347 stars 69 forks source link

Remove /xo argument from robocopy filesystem deployments #96

Closed divsyd closed 7 years ago

divsyd commented 7 years ago

Hello,

The expected behavior from Robocopy mirror deployments is to mirror the source DIR to the destination but i found that if files are updated in the source, they aren't replaced. This is due to the /xo flag. I tested without this and confirmed any changes in destination are replaced with files from source once this is removed.

This ensures staff are not updating PROD and PROD will always mirror our source git repository when using psdeploy.

Robocopy /? /XO : eXclude Older - if destination file exists and is the same date or newer than the source - don’t bother to overwrite it.

RamblingCookieMonster commented 7 years ago

Hiyo! Good catch! Any chance you would swap this so that /XO is there, but only when Mirror isn't specified? For example, maybe move the /XO to an else statement here?

Cheers!

divsyd commented 7 years ago

Hi @RamblingCookieMonster

I think this will have the same issue? e.g. If I do a deploy be it copy or mirror, I'd expect the dest files to match the source files. But with the XO flag If someone updates the dest files, then they will be skipped because they are newer than source.

Please see test example here: https://gist.github.com/divsyd/71cfdbeb09cf2463045f87919dbb4531

Cheers

mirraxian commented 7 years ago

I ran into issues with this before seeing this thread. I solved this by adding an additional Deployment option for the FileSystem provider: IncludeOlder. This way it won't change existing behavior. Here's a fork with how I handled it: https://github.com/BenHimsel/PSDeploy/blob/master/PSDeploy/PSDeployScripts/FileSystem.ps1 Let me know if you like this approach better and I'll create a pull request.

RamblingCookieMonster commented 7 years ago

@divsyd - sorry for the delay, last day of vacation! Totally agree with you in theory - problem is that a number of deployments use this as is - would prefer a non-breaking change.

I think @BenHimsel's approach makes sense, although it might still be worth removing /XO when mirror is specified, otherwise it's a bit of a misnomer?

Cheers!

divsyd commented 7 years ago

Looks good, thanks!