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

Fix cache directory on windows #251

Merged
merged 6 commits into from
Nov 30, 2016
Merged

Conversation

afiune
Copy link

@afiune afiune commented Nov 30, 2016

We are going to default to C Drive since vagrant does not know about
the environament variables of the guest VM.

Closes #250

Signed-off-by: Salim Afiune [email protected]

We are going to default to `C` Drive since vagrant does not know about
the environament variables of the guest VM.

Closes #250

Signed-off-by: Salim Afiune <[email protected]>
@afiune
Copy link
Author

afiune commented Nov 30, 2016

cc/ @test-kitchen/maintainers

@coderanger
Copy link
Contributor

Not sure why this is needed, all the install management script stuffs are run on the instance, not locally. Also these two paths aren't equivalent, if you want to use a temp folder.

@afiune
Copy link
Author

afiune commented Nov 30, 2016

@coderanger You are right! 😄 But the generation of the Vagrantfile is not and vagrant can't resolve $env:TEMP from the host.

@coderanger
Copy link
Contributor

Hard coding C: is usually something we regret, so if it has to be static I would at least put in some way to configure it.

@cheeseplus
Copy link
Contributor

I like the idea of at least making it configurable - that way we can fix the default case but also not dig ourselves a future hole.

@afiune
Copy link
Author

afiune commented Nov 30, 2016 via email

Salim Afiune added 4 commits November 30, 2016 11:44
Lets make the `cache_directory` configurable so that if the end-user
needs to customize it it can set this on their kitchen.yml file:
```
---
driver:
  cache_directory: D:\\my\\custom\\cache
```

Signed-off-by: Salim Afiune <[email protected]>
Travis is complaining with a deprecation, this commit simple applies the
recomendations from:
https://github.com/codeclimate/ruby-test-reporter/blob/master/CHANGELOG.md#v100-2016-11-03

Signed-off-by: Salim Afiune <[email protected]>
@cheeseplus
Copy link
Contributor

I think those last few commits fix up @coderanger's points and I'm a big +1

@afiune
Copy link
Author

afiune commented Nov 30, 2016

Last quick commit to fix the pipeline.

Signed-off-by: Salim Afiune <[email protected]>
@afiune
Copy link
Author

afiune commented Nov 30, 2016

ping @coderanger ^^^

Copy link
Contributor

@coderanger coderanger left a comment

Choose a reason for hiding this comment

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

I assume the simplecov stuff is because of the compat break with code-climate, the actual change looks good though 👍

@afiune
Copy link
Author

afiune commented Nov 30, 2016

@kernelsmith
Copy link

I actually tried this change manually and it didn't work, seems like the cache string is getting interpreted twice. I had to use c:\\omnibus\\cache otherwise it continued to present as c:omnibusche

 default: -- /Users/me/.kitchen/cache: C:omnibusche
STDERR: The following WinRM command responded with a non-zero exit status.
Vagrant assumes that this means the command failed!

function Test-ReparsePoint([string]$path) {
  $file = Get-Item $path -Force -ea 0
  return [bool]($file.Attributes -band [IO.FileAttributes]::ReparsePoint)
}

$MountPoint = [System.IO.Path]::GetFullPath("C:omnibusche")
$ShareName = "C:omnibusche"
$VmProviderUncPath = "\\vmware-host\Shared Folders\C:omnibusche"

@kernelsmith
Copy link

also, %SYSTEMDRIVE% instead of C: may avoid future pains, tho may require some additional escaping

@afiune
Copy link
Author

afiune commented Dec 5, 2016

@kernelsmith Thank you for reporting your issue, we have fixed this problem and we are now in the process of releasing a new gem. If you want to try it you can always use the latest code in master this repository.

@tas50 tas50 deleted the afiune/250/fix-windows-cache-dir branch January 30, 2022 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants