Relative symlinks don't work with --device argument#22272
Merged
vdemeester merged 1 commit intomoby:masterfrom Apr 25, 2016
Merged
Relative symlinks don't work with --device argument#22272vdemeester merged 1 commit intomoby:masterfrom
vdemeester merged 1 commit intomoby:masterfrom
Conversation
Member
|
@mlaventure we may want to consider including this in 1.11.1 (the follow symlinks is a new addition in 1.11.0) |
Contributor
There was a problem hiding this comment.
Does os.Symlink error out if /dev/symzero already exists? if so, i dont think we should error out, because it is not impossible that the defer won't run in some weird cases. We could just print the error though and move on.
Member
Author
There was a problem hiding this comment.
Thanks @tiborvass just updated the pull request. Let me know if there are any other issues.
Contributor
|
I'm not opposed to this PR, in fact I don't even see why we wouldn't follow absolute symlinks too. |
yongtang
force-pushed
the
22271-relative-symlinks
branch
2 times, most recently
from
7731ab5 to
0608629
Compare
Member
|
Same thought as @tiborvass so LGTM 🐼 |
Contributor
|
one nit, otherwise LGTM |
Contributor
There was a problem hiding this comment.
should defer os.Remove("/dev/symzero") below the if err != nil check?
yongtang
force-pushed
the
22271-relative-symlinks
branch
2 times, most recently
from
36c1c37 to
a73e631
Compare
This fix tries to address the issue raised in moby#22271 where relative symlinks don't work with --device argument. Previously, the symlinks in --device was implemneted (moby#20684) with `os.Readlink()` which does not resolve if the linked target is a relative path. In this fix, `filepath.EvalSymlinks()` has been used which will reolve correctly with relative paths. An additional test case has been added to the existing `TestRunDeviceSymlink` to cover changes in this fix. This fix is related to moby#13840 and moby#20684, moby#22271. This fix fixes moby#22271. Signed-off-by: Yong Tang <[email protected]>
yongtang
force-pushed
the
22271-relative-symlinks
branch
from
a73e631 to
632b314
Compare
Contributor
|
LGTM |
Merged
liusdu
pushed a commit
to liusdu/moby
that referenced
this pull request
Oct 30, 2017
This fix tries to address the issue raised in moby#22271 where relative symlinks don't work with --device argument. Previously, the symlinks in --device was implemneted (moby#20684) with `os.Readlink()` which does not resolve if the linked target is a relative path. In this fix, `filepath.EvalSymlinks()` has been used which will reolve correctly with relative paths. An additional test case has been added to the existing `TestRunDeviceSymlink` to cover changes in this fix. backport from: moby#22272 Signed-off-by: Yong Tang <[email protected]> Signed-off-by: Yuanhong Peng <[email protected]>
liusdu
pushed a commit
to liusdu/moby
that referenced
this pull request
Oct 30, 2017
Relative symlinks don't work with --device argument This fix tries to address the issue raised in moby#22271 where relative symlinks don't work with --device argument. Previously, the symlinks in --device was implemneted (moby#20684) with `os.Readlink()` which does not resolve if the linked target is a relative path. In this fix, `filepath.EvalSymlinks()` has been used which will reolve correctly with relative paths. An additional test case has been added to the existing `TestRunDeviceSymlink` to cover changes in this fix. backport from: moby#22272 Signed-off-by: Yong Tang <[email protected]> Signed-off-by: Yuanhong Peng <[email protected]> See merge request docker/docker!348
This was referenced Oct 7, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
This fix tries to address the issue raised in #22271 where relative symlinks don't work with --device argument.
- How I did it
Previously, the symlinks in --device was implemneted (#20684) with
os.Readlink()which does not resolve if the linked target is a relative path. In this fix,filepath.EvalSymlinks()has been used which will reolve correctly with relative paths.- How to verify it
An additional test case has been added to the existing
TestRunDeviceSymlinkto cover changes in this fix.- A picture of a cute animal (not mandatory but encouraged)
This fix is related to #13840 and #20684, #22271.
This fix fixes #22271.
Signed-off-by: Yong Tang [email protected]