Skip to content
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

Make solo tarballs work on windows; fixes #1515 #1751

Closed
wants to merge 2 commits into from

Conversation

adamhjk
Copy link
Contributor

@adamhjk adamhjk commented Aug 5, 2014

Adds Guard support to Chef.

Makes solo tarballs work on windows, by converting to ShellOut and adding smarter path separator support to the regex that detects the cookbook directory to untar into.

Adds proper path seperator support to the tarball path, and converts the
legacy call to tar via run_command to the Windows-tastic ShellOut. Adds
convenience functions for ShellOut helpers being called directly, a-la
run_command.

Tests are incomplete, since getting the system to behave as if it was
on windows means mocking out significant path functionality for dubious
results.
@adamhjk
Copy link
Contributor Author

adamhjk commented Aug 5, 2014

This code needs to be manually tested on windows, because writing unit tests quickly turns into a festival of mockitudes.

@@ -72,6 +72,14 @@ def self.platform_path_separator
end
end

def self.platform_path_separator_escaped
Copy link
Contributor

Choose a reason for hiding this comment

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

@btm, should this use one of the new helpers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not here specifically, since this just returns special characters as a separator. But the changes in lib/chef/application/solo.rb could probably use Chef::Util::PathHelper.validate_path.

Also, we're not using Chef::Config.platform_path_separator in other classes yet, we probably shouldn't start. Consider putting this in Chef::Util::PathHelper with a replacement for platform_path_separator that we could use to replace the multiple places in the code that we do ::File::ALT_SEPARATOR ? ::File::ALT_SEPARATOR : ::File::SEPARATOR currently. Dan also mentioned lib/chef/platform/query_helpers.rb as a potential place to consider for this new method.

@zl4bv
Copy link

zl4bv commented Dec 4, 2014

Is there currently anything that is preventing this PR from being merged?

@lamont-granquist
Copy link
Contributor

@adamhjk if you could address dan's and btm's comments and rebase that'd be cool.

I believe we also picked up the tarball stuff into chef-client -z recently as well, so it probably needs to get fixed in two places now.

@lamont-granquist
Copy link
Contributor

If this hasn't been fixed and still needs fixing, we should probably fix it

@thommay
Copy link
Contributor

thommay commented Feb 9, 2016

Let's get this done ourselves.

lamont-granquist added a commit that referenced this pull request Feb 10, 2016
- 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
lamont-granquist added a commit that referenced this pull request Feb 10, 2016
@thommay thommay added Type: Bug Does not work as expected. Status: Finish for Merge and removed Bug labels Jan 25, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Windows Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet