command: check for wsl mount path on windows #5424
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5424 +/- ##
==========================================
+ Coverage 59.73% 59.75% +0.02%
==========================================
Files 345 345
Lines 23394 23428 +34
==========================================
+ Hits 13974 13999 +25
- Misses 8450 8455 +5
- Partials 970 974 +4 |
jsternberg
force-pushed
the
wsl-socket-path
branch
4 times, most recently
from
3045f17
to
cb36c1d
Compare
| // Enforce that we are using forward slashes. | ||
| return "unix://" + filepath.ToSlash(socketPath) |
There was a problem hiding this comment.
Should this be an error (or warning -> ignore) instead? i.e., should we only accept unix:// and only accept forward-slashes?
There was a problem hiding this comment.
This is mostly just to make sure the output is correct. I don't think this needs to be an error or warning. The reason why it's here and not somewhere else is it communicates that the requirement for forward slashes comes from the URL format. We could move this call into wslSocketPath and it would be fine, but we'd want to add a comment along the lines of "using forward slashes so this is compatible with a URL" so why not just have the code where we're constructing the URL?
There was a problem hiding this comment.
IIUC, the unix:// + here isn't to just add the prefix back in @thaJeztah, it's just re-adding the prefix we cut out when we parsed the URL/transformed the path.
There was a problem hiding this comment.
Ah, right; overlooked that part.
I guess we could've written back to u.Path, and returned u.String() 😅
Either way; it's probably ok for now; we can do follow-ups
This checks for the equivalent WSL mount path on windows. WSL will mount the windows drives at `/mnt/c` (or whichever drive is being used). This is done by parsing a UNC path with forward slashes from the unix socket URL. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
jsternberg
force-pushed
the
wsl-socket-path
branch
from
cb36c1d
to
38c3fef
Compare
|
To reiterate (and make reviewing easier), the behavior is now:
(cc @jsternberg) |
| // Enforce that we are using forward slashes. | ||
| return "unix://" + filepath.ToSlash(socketPath) |
There was a problem hiding this comment.
Ah, right; overlooked that part.
I guess we could've written back to u.Path, and returned u.String() 😅
Either way; it's probably ok for now; we can do follow-ups
- What I did
This checks for the equivalent WSL mount path on windows. WSL will mount the windows drives at
/mnt/c(or whichever drive is being used).- How I did it
The code will now check to see if a file exists in
/mnt/<drive>. If there is no drive in the path, it'll default to the C drive. If that file exists, it will use it. The check now also supports URLs of the formatc:/path/to/file(note the forward slashes rather than backslashes).- How to verify it
Integrate the change with buildx and run a build in a WSL container on Windows.
- Description for the changelog
* Properly report metrics when run in WSL environment on Windows