Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

improve cmd shell support #1493

Closed
wants to merge 2 commits into from
Closed

improve cmd shell support #1493

wants to merge 2 commits into from

Conversation

StefanScherer
Copy link
Member

After writing down a proposal in #1033 to improve the cmd.exe shell support of docker-machine I have added it to the sourcecode.

With this change the user has a command how to set the environments in the current shell.

C:\> docker-machine.exe env --shell=cmd dev
set DOCKER_TLS_VERIFY=1
set DOCKER_HOST=tcp://192.168.1.123:2376
set DOCKER_CERT_PATH=/Users/stefan/.docker/machine/machines/dev
set DOCKER_MACHINE_NAME=dev
rem Run this command to configure your shell: 
rem for /f "tokens=*" %i in ('docker-machine_darwin-amd64 env --shell=cmd dev') do %i

I also have cleaned up the code a little bit to reduce the if-else blocks to a minimum.

Signed-off-by: Stefan Scherer scherer_stefan@icloud.com

Signed-off-by: Stefan Scherer <scherer_stefan@icloud.com>
@ahmetb
Copy link
Contributor

ahmetb commented Jul 8, 2015

why the new line after DOCKER_CERT_PATH?

Also I'm thinking if we should make console cmds uppercase like SET REM..

@ahmetb
Copy link
Contributor

ahmetb commented Jul 8, 2015

Also how about adding a \t before the usage command line:

REM How to run this:
REM     for bla bla bla

Signed-off-by: Stefan Scherer <scherer_stefan@icloud.com>
@StefanScherer
Copy link
Member Author

@ahmetalpbalkan Now it looks like this:

C:\Windows\system32>docker-machine env dev --shell=cmd
SET DOCKER_TLS_VERIFY=1
SET DOCKER_HOST=tcp://192.168.99.100:2376
SET DOCKER_CERT_PATH=C:\Users\vagrant\.docker\machine\machines\dev
SET DOCKER_MACHINE_NAME=dev
REM Run this command to configure your shell:
REM     FOR /f "tokens=*" %i IN ('C:\ProgramData\chocolatey\lib\docker-machine\bin\docker-machine.exe env --shell=cmd dev') DO %i

@StefanScherer
Copy link
Member Author

Here is a complete walkthrough:

bildschirmfoto 2015-07-09 um 00 52 12

@ahmetb
Copy link
Contributor

ahmetb commented Jul 8, 2015

Hmm do we need to print the full docker-machine.exe path in the screenshot? We can just use args[0] maybe?

@StefanScherer
Copy link
Member Author

Yes, just had the same idea. I've tried with os.Args[0] but the output is still the same:

C:\Users\vagrant>docker-machine env dev --shell=cmd
SET DOCKER_TLS_VERIFY=1
SET DOCKER_HOST=tcp://192.168.99.100:2376
SET DOCKER_CERT_PATH=C:\Users\vagrant\.docker\machine\machines\dev
SET DOCKER_MACHINE_NAME=dev
REM Run this command to configure your shell:
REM     FOR /f "tokens=*" %i IN ('C:\ProgramData\chocolatey\lib\docker-machine\bin\docker-machine.exe env --shell=cmd dev') DO %i

Then I put the exe into another directory that is in the PATH and here it works:

C:\Users\vagrant>mkdir bin

C:\Users\vagrant>copy \vagrant\resources\docker-machine_windows-amd64.exe bin\docker-machine.exe
        1 file(s) copied.

C:\Users\vagrant>set PATH=bin;%PATH%

C:\Users\vagrant>docker-machine env dev --shell=cmd
SET DOCKER_TLS_VERIFY=1
SET DOCKER_HOST=tcp://192.168.99.100:2376
SET DOCKER_CERT_PATH=C:\Users\vagrant\.docker\machine\machines\dev
SET DOCKER_MACHINE_NAME=dev
REM Run this command to configure your shell:
REM     FOR /f "tokens=*" %i IN ('docker-machine env --shell=cmd dev') DO %i

It seems that has something to do with the ShimGen.exe of Chocolatey that calls the real docker-machine.exe with absolute path and so Args[0] has the complate path in it.

I don't like to add workarounds in docker-machine for this.

@StefanScherer
Copy link
Member Author

Same thing for powershell. Chocolatey's redirect puts in the absolute path:

PS C:\Users\vagrant> docker-machine env dev --shell=powershell
$Env:DOCKER_TLS_VERIFY = "1"
$Env:DOCKER_HOST = "tcp://192.168.99.100:2376"
$Env:DOCKER_CERT_PATH = "C:\Users\vagrant\.docker\machine\machines\dev"
$Env:DOCKER_MACHINE_NAME = "dev"
# Run this command to configure your shell:
# C:\ProgramData\chocolatey\lib\docker-machine\bin\docker-machine.exe env --shell=powershell dev | Invoke-Expression
PS C:\Users\vagrant>

@ahmetb
Copy link
Contributor

ahmetb commented Jul 9, 2015

Ah true. It's the chocolatey shimgen exec'ing with the full path...

@ahmetb
Copy link
Contributor

ahmetb commented Jul 9, 2015

LGTM (not a maintainer).

@nathanleclaire
Copy link
Contributor

I will have to test this out soon and get back to you.

@ahmetb
Copy link
Contributor

ahmetb commented Aug 13, 2015

any updates on this?

@dmp42
Copy link
Contributor

dmp42 commented Oct 21, 2015

@nathanleclaire what's the status on this?

@nathanleclaire
Copy link
Contributor

I'm interested, but it needs tests. The good news is that I think @dgageot has begun some test support for the usage hints in: #2027

So, let's work on getting that merged, then rebase this and add some tests, then it'll be good to go 👍

@dgageot
Copy link
Member

dgageot commented Oct 21, 2015

@nathanleclaire @dmp42 I can work on it tomorrow

@nathanleclaire
Copy link
Contributor

SGTM @dgageot 👍

@StefanScherer
Copy link
Member Author

👍

@dgageot
Copy link
Member

dgageot commented Oct 22, 2015

@StefanScherer @nathanleclaire the replacement PR with unit tests is #2058

@StefanScherer
Copy link
Member Author

PR #2058 merged, so I close this one. Thanks!

@StefanScherer StefanScherer deleted the improve-cmd-exe-support branch October 22, 2015 19:04
@nathanleclaire
Copy link
Contributor

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants