Skip to content

[18.09 backport] Detect Windows absolute paths on non-Windows CLI - #1994

Merged
kolyshkin merged 1 commit into
docker:18.09from
thaJeztah:18.09_backport_cross_platform_bind
Jul 23, 2019
Merged

[18.09 backport] Detect Windows absolute paths on non-Windows CLI#1994
kolyshkin merged 1 commit into
docker:18.09from
thaJeztah:18.09_backport_cross_platform_bind

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

backport of #1990

Note that there is an alternative PR in #1871, but that PR changes the current behaviour, and might require more discussion. This PR tries to focus on just the basic problem.

addresses #1403
addresses moby/moby#33746
fixesmoby/moby#34810

When deploying a stack using a relative path as bind-mount source in the compose file, the CLI converts the relative path to an absolute path, relative to the location of the docker-compose file.

This causes a problem when deploying a stack that uses an absolute Windows path, because a non Windows client will fail to detect that the path (e.g. C:\somedir) is an absolute path (and not a relative directory named C:\).

The existing code did already take Windows clients deploying a Linux stack into account (by checking if the path had a leading slash). This patch adds the reverse, and adds detection for Windows absolute paths on non-Windows clients.

The code used to detect Windows absolute paths is copied from the Golang filepath package;
https://github.com/golang/go/blob/1d0e94b1e13d5e8a323a63cd1cc1ef95290c9c36/src/path/filepath/path_windows.go#L12-L65

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

When deploying a stack using a relative path as bind-mount
source in the compose file, the CLI converts the relative
path to an absolute path, relative to the location of the
docker-compose file.
This causes a problem when deploying a stack that uses
an absolute Windows path, because a non-Windows client will
fail to detect that the path (e.g. `C:\somedir`) is an absolute
path (and not a relative directory named `C:\`).
The existing code did already take Windows clients deploying
a Linux stack into account (by checking if the path had a leading
slash). This patch adds the reverse, and adds detection for Windows
absolute paths on non-Windows clients.
The code used to detect Windows absolute paths is copied from the
Golang filepath package;
https://github.com/golang/go/blob/1d0e94b1e13d5e8a323a63cd1cc1ef95290c9c36/src/path/filepath/path_windows.go#L12-L65
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d6dd08d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #1994 into 18.09 will increase coverage by 0.05%.
The diff coverage is 90.32%.

@@ Coverage Diff @@## 18.09 #1994 +/- ##
==========================================
+ Coverage 54.23% 54.29% +0.05% 
==========================================
Files 290 291 +1 Lines 19428 19458 +30 ==========================================
+ Hits 10537 10564 +27 - Misses 8212 8214 +2 - Partials 679 680 +1

1 similar comment
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #1994 into 18.09 will increase coverage by 0.05%.
The diff coverage is 90.32%.

@@ Coverage Diff @@## 18.09 #1994 +/- ##
==========================================
+ Coverage 54.23% 54.29% +0.05% 
==========================================
Files 290 291 +1 Lines 19428 19458 +30 ==========================================
+ Hits 10537 10564 +27 - Misses 8212 8214 +2 - Partials 679 680 +1

@silvin-lubeckisilvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@vdemeestervdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🐯

@thaJeztahthaJeztah modified the milestones: 18.09.8, 18.09.9Jul 18, 2019
@thaJeztahthaJeztah changed the title [WIP][18.09 backport] Detect Windows absolute paths on non-Windows CLI[18.09 backport] Detect Windows absolute paths on non-Windows CLIJul 23, 2019
@kolyshkin
kolyshkin merged commit 3d0a1f6 into docker:18.09Jul 23, 2019
@thaJeztah
thaJeztah deleted the 18.09_backport_cross_platform_bind branch July 23, 2019 23:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@thaJeztah@codecov-io@vdemeester@silvin-lubecki@kolyshkin@GordonTheTurtle