-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
(MAINT) fix paths when using DOCKER_TOOLBOX on windows #7
Conversation
a = [ File.expand_path(mount['host_path']), mount['container_path'] ] | ||
host_path = File.expand_path(mount['host_path']) | ||
# When using docker_toolbox and getting a "(Driveletter):/" path, convert windows path to VM mount | ||
if ENV['DOCKER_TOOLBOX_INSTALL_PATH'] && host_path =~ /^.\:\// |
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.
Haven't used Docker Toolbox, is DOCKER_TOOLBOX_INSTALL_PATH
a standard environment variable set up by the installer?
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.
Yeps, installer sets that https://github.com/docker/toolbox/blob/c971a9cb9b35f5bea1d3724702423135b530a226/windows/Toolbox.iss#L91
Its also used when running the "docker shell" executable - so should stay arround https://github.com/docker/toolbox/blob/master/windows/start.sh
With the path check /^.\:\//
I belive its fairly safe to use for anyone...
-> mac has docker_toolbox too
Only acts on C:/
not C:\
so when using msys/gitbash
-> should not affect cmd/powershell (windows-on-windows-dev
?)
Having "native and toolbox" installed breaks a bunch of other things,
->so likely "should not be our problem"
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.
Looks good to me, only activates when Docker Toolbox is installed.
@tabakhase one quick contributor standard item before moving forward: please prepend your git commit message with Once that's done, we'll re-kick testing to push it through the automation and towards merging. Thanks for the fix! |
Turns `D:/devstuff` into `/d/devstuff` when detecting DOCKER_TOOLBOX (docker inside virtualbox on windows) fixes `Docker::Error::ServerError: invalid volume specification: 'D:/devstuff:/tmp/stuff'`
@kevpl PR & commit have been updated. |
Jenkins, test this please |
test this please |
Can one of the admins verify this patch? |
Turns
D:/devstuff
into/d/devstuff
when detecting DOCKER_TOOLBOX (docker inside virtualbox on windows)fixes
Docker::Error::ServerError: invalid volume specification: 'D:/devstuff:/tmp/stuff'
I assume thats by far not the elegant way todo that (rubynoob), so feel free to edit.