removing rm -rf in chef-solo recipe_url

[lamont-granquist]

• deprecates '-r' used for the recipe_url in chef-solo
• adds --delete-entire-chef-repo option for users who want the
  old behavior back.
• cleans up some old code

closes #3802
closes #1515
closes #1751

[btm]

alternative for consideration, exit if the thing passed isn't a valid url?

[lamont-granquist]

We don't do the rm_rf in the client code, and in order to synchronize the 'features' i strongly vote that the client version of this code wins -- either now, or in 13.0

[lamont-granquist]

see #3802

[lamont-granquist]

also the lack of stubs around the rm_rf in the unit tests blew away my chef github checkout once here: #2810

[lamont-granquist]

So, it has trolled me twice and destroyed data -- only one of which times it would have been avoided by the 'check for a real url', and its trolled @ranjib and @gionn when using it 'correctly' as well. The 'feature' is really indefensible.

See also ValveSoftware/steam-for-linux#3671

[thommay]

I would be in favour of removing the rm -rf now, and then dumping the -r argument in 13.

[danielsdeleo]

The way it behaves now is terri-bad and I'm all for changing it. I just wonder how we even got to this situation. Did we change something in the config to make local mode work better and now recipes_path defaults to pwd where it didn't before? Can we dig up the reason this rm_rf exists in the first place? I'm 👍 on this now but would be + 💯 if we could figure out the point of this code and find a good way of dealing with whatever the original problem was (e.g., extract the tarball to a cache dir and fixup Chef::Config's paths). I would also be +90 if we could at least detect the failure mode ahead of time and give the user an explanation instead of something like shell out failing on tar zxvf. This code in ChefDK is kinda along the lines of what I'm trying to describe: https://github.com/chef/chef-dk/blob/23d880e44dc36a60ea036f57ff44850418639abb/lib/chef-dk/policyfile_services/export_repo.rb#L339

[lamont-granquist]

see #2400

i forgot that i was the one that merged this...

shell_out doesn't fail on the untar, but if there's already a cookbooks dir that you're extracting over then odd things could happen.

[lamont-granquist]

@chef/client-core okay this is a bit less angry now

• the -r option to solo is deprecated and just complains to users to use --recipe-url
• a --delete-entire-chef-repo is added (with no short option) to do the old FileUtils.rm_rf, users that want the old behavior can add it back with this option
• also uses cleanpath to fix the windows path issue in --recipe-url due to multiple cookbook paths that has been kicking around since forever

[lamont-granquist]

also a bit of cleanup to use shell_out! more consistently, etc. there's probably a common method to extract here and put it in the superclass, but at the same time the local mode check and the cookbook-array junk in chef-solo make it different enough that i think i've spent enough time on this bit of code for now...

[btm]

👍

[danielsdeleo]

👍 modulo whatever travis trolling is going on

[lamont-granquist]

s/travis trolling/rubocop trolling/