Skip to content

[19.03 backport] Detect Windows absolute paths on non-Windows CLI - #1995

Merged
thaJeztah merged 1 commit into
docker:19.03from
thaJeztah:19.03_backport_cross_platform_bind
Jul 26, 2019
Merged

[19.03 backport] Detect Windows absolute paths on non-Windows CLI#1995
thaJeztah merged 1 commit into
docker:19.03from
thaJeztah:19.03_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 #1995 into 19.03 will increase coverage by 0.04%.
The diff coverage is 90.32%.

@@ Coverage Diff @@## 19.03 #1995 +/- ##
==========================================
+ Coverage 56.71% 56.75% +0.04% 
==========================================
Files 310 311 +1 Lines 21789 21819 +30 ==========================================
+ Hits 12357 12384 +27 - Misses 8519 8521 +2 - Partials 913 914 +1

1 similar comment
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #1995 into 19.03 will increase coverage by 0.04%.
The diff coverage is 90.32%.

@@ Coverage Diff @@## 19.03 #1995 +/- ##
==========================================
+ Coverage 56.71% 56.75% +0.04% 
==========================================
Files 310 311 +1 Lines 21789 21819 +30 ==========================================
+ Hits 12357 12384 +27 - Misses 8519 8521 +2 - Partials 913 914 +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

@thaJeztahthaJeztah changed the title [WIP][19.03 backport] Detect Windows absolute paths on non-Windows CLI[19.03 backport] Detect Windows absolute paths on non-Windows CLIJul 11, 2019

@kolyshkinkolyshkin 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

@thaJeztahthaJeztah modified the milestones: 19.03.0, 19.03.1Jul 18, 2019
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

moving this to the 19.03.1 milestone (don't merge yet)

@thaJeztahthaJeztah modified the milestones: 19.03.1, 19.03.2Jul 26, 2019
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

this is ok to merge now

@thaJeztah
thaJeztah merged commit d473c60 into docker:19.03Jul 26, 2019
@thaJeztah
thaJeztah deleted the 19.03_backport_cross_platform_bind branch July 28, 2019 14:36
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.

5 participants

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