Support parsing of named pipes for compose volumes#560
Support parsing of named pipes for compose volumes#560vdemeester merged 1 commit intodocker:masterfrom
Conversation
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Codecov Report
@@ Coverage Diff @@
## master #560 +/- ##
==========================================
+ Coverage 49.5% 49.51% +<.01%
==========================================
Files 208 208
Lines 17153 17156 +3
==========================================
+ Hits 8491 8494 +3
Misses 8230 8230
Partials 432 432 |
StefanScherer
left a comment
There was a problem hiding this comment.
Thanks for starting this PR.
To have Docker as a tool that gets out of the users way please support (unix) slashes as well. ❤️ 🙏 😃
| } | ||
|
|
||
| // windows named pipes | ||
| if strings.HasPrefix(source, `\\`) { |
There was a problem hiding this comment.
I wonder if we can make this slash-insensitive (like case-insensitive, but for slashes ;-) ).
From a Mac/Linux client remote controlling a Windows swarm node I guess many people still are used to unix slashes.
I also use eg. docker run -v //./pipe/docker_engine://./pipe/docker_engine -it -p 8080:8080 stefanscherer/visualizer-windows:insider right now and it works pretty good as Golang itself can handle backslashes and slashes in paths.
Slashes because bash needs extra escaped backslashes as you can see here.
´´´
$ docker run -v \.\pipe\docker_engine:\.\pipe\docker_engine -it -p 8080:8080 stefanscherer/visualizer-windows:insider
docker: Error response from daemon: invalid bind mount spec "\.pipedocker_engine:\.pipedocker_engine": invalid volume specification: '.pipedocker_engine:.pipedocker_engine'.
(oops as you can see even GitHub looses some backslashes although I'm using three backticks here)
There was a problem hiding this comment.
Normal slash is already handled in this function on the line above (111). A single / already returns true.
|
It's so convenient to pick the Docker CLI from CircleCI build. So I gave it a try And with backslashes running against a Windows Insider with Docker engine 17.09.0-rc2 installed. Is this another check in cli or in engine? |
|
Yes, as I mentioned in moby/moby#34795 (comment), this only fixes the client side. |
Related to moby/moby#34795
Fixes the client side parsing of volume specs to support windows named pipes.